Add new SCRIPT SHOW subcommand to dump script via sha1

[kukey]

In some scenarios, the business may not be able to find the
previously used Lua script and only have a SHA signature.
Or there are multiple identical evalsha's args in monitor/slowlog,
and admin is not able to distinguish the script body.

Add a new script subcommmand to show the contents of script
given the scripts sha1. Returns a NOSCRIPT error if the script
is not present in the cache.

Usage: SCRIPT SHOW sha1
Complexity: O(1)

Closes #604.
Doc PR: valkey-io/valkey-doc#143

[enjoy-binbin]

please also update the top comment instead of directly linking to an issue

[codecov]

Codecov Report

Attention: Patch coverage is 77.77778% with 2 lines in your changes missing coverage. Please review.

Project coverage is 70.02%. Comparing base (71dd85d) to head (e8c0d1f).
Report is 23 commits behind head on unstable.

Additional details and impacted files

@@             Coverage Diff              @@
##           unstable     #617      +/-   ##
============================================
- Coverage     70.09%   70.02%   -0.07%     
============================================
  Files           110      110              
  Lines         60041    60076      +35     
============================================
- Hits          42085    42068      -17     
- Misses        17956    18008      +52     

Files	Coverage Δ
src/commands.def	100.00% <ø> (ø)
src/server.c	88.54% <100.00%> (-0.39%)	⬇️
src/eval.c	56.60% <75.00%> (+0.53%)	⬆️

... and 32 files with indirect coverage changes

[madolson]

@valkey-io/core-team Vote on adding this new command which allows dumping a specific command. There is still some minor open comment about error wording, but other than that it is broadly updated.

[madolson]

@kukey Btw, you need to regenerate the commands.def and check it in.

[madolson]

Looks OK to me. Waiting on other for core team approval before merging.

[madolson]

Once we have directional approval for the merge, please create a PR to the doc repo similar to https://github.com/valkey-io/valkey-doc/blob/main/commands/script-exists.md.

[enjoy-binbin]

@valkey-io/core-team I think this one is ready to merge, i updated the top comment, please take a look.

[soloestoy]

I like this idea, this can help users to debug. But the subcommand's name SCRIPT DUMP is a bit wired to me, since other commands like DUMP and FUNCTION DUMP's reply is rdb format, but this subcommand's reply is just raw. Maybe SCRIPT GET or SCRIPT FETCH is better.

[madolson]

I like this idea, this can help users to debug. But the subcommand's name SCRIPT DUMP is a bit wired to me, since other commands like DUMP and FUNCTION DUMP's reply is rdb format, but this subcommand's reply is just raw. Maybe SCRIPT GET or SCRIPT FETCH is better.

I don't feel strongly. SCRIPT GET would be my preferred option of the two. I guess I would prefer something like SCRIPT DISPLAY.

[zuiderkwast]

Regarding subcommand name, how about SCRIPT SHOW?

Compare: In MySQL it's possible to get the CREATE TABLE command for a table using SHOW CREATE TABLE mytable.

[madolson]

Regarding subcommand name, how about SCRIPT SHOW?

I considered that but it just sort of sounds weird to me, but I would prefer that over SCRIPT GET and SCRIPT FETCH.

[hwware]

Regarding subcommand name, how about SCRIPT SHOW?

I considered that but it just sort of sounds weird to me, but I would prefer that over SCRIPT GET and SCRIPT FETCH.

script fetch is much more officially.

[zuiderkwast]

Fetch sounds like "go and get it from somewhere", like from another node or file or something. Another idea: SCRIPT SOURCE.

[kukey]

Once we have directional approval for the merge, please create a PR to the doc repo similar to https://github.com/valkey-io/valkey-doc/blob/main/commands/script-exists.md.

@madolson valkey-io/valkey-doc#143

[kukey]

I prefer SCRIPT GET, it's clear and simple. @valkey-io/core-team which subcommand we select? Or also continue use SCRIPT DUMP？

[PingXie]

Regarding subcommand name, how about SCRIPT SHOW?

my vote ^

[madolson]

@kukey We have an online syncup tomorrow, I think we can quickly get consensus since people's opinions seem all over the place online.

[madolson]

@kukey SCRIPT SHOW Seems to be the winner from our online discussion.

[kukey]

@kukey SCRIPT SHOW Seems to be the winner from our online discussion.

copy

[madolson]

@zuiderkwast I've been using the needs-doc-pr to just track if it's been opened. The doc PR already exists here: valkey-io/valkey-doc#143. Is this inline with what you were thinking?

[zuiderkwast]

OK, sounds good. I didn't notice we remove it before. In redis it was never removed I think.

I missed the link to the doc PR. Can we add the link in the PR description to make it more visible?