Describe the bug
A short description of the bug.
127.0.0.1:6379> FUNCTION LOAD Lua mylib desc noop "redis.register_function('myfunc', function(keys, args) return args[1] end)"
(error) ERR Unknown option given: desc
but function help show that the desc attribute
127.0.0.1:6379> function help
1) FUNCTION <subcommand> [<arg> [value] [opt] ...]. Subcommands are:
2) LOAD <ENGINE NAME> <LIBRARY NAME> [REPLACE] [DESC <LIBRARY DESCRIPTION>] <LIBRARY CODE>
3) Create a new library with the given library name and code.
4) DELETE <LIBRARY NAME>
5) Delete the given library.
6) LIST [LIBRARYNAME PATTERN] [WITHCODE]
7) Return general information on all the libraries:
8) * Library name
9) * The engine used to run the Library
10) * Library description
11) * Functions list
12) * Library code (if WITHCODE is given)
13) It also possible to get only function that matches a pattern using LIBRARYNAME argument.
14) STATS
15) Return information about the current function running:
16) * Function name
17) * Command used to run the function
18) * Duration in MS that the function is running
19) If no function is running, return nil
20) In addition, returns a list of available engines.
21) KILL
22) Kill the current running function.
23) FLUSH [ASYNC|SYNC]
24) Delete all the libraries.
25) When called without the optional mode argument, the behavior is determined by the
26) lazyfree-lazy-user-flush configuration directive. Valid modes are:
27) * ASYNC: Asynchronously flush the libraries.
28) * SYNC: Synchronously flush the libraries.
29) DUMP
30) Returns a serialized payload representing the current libraries, can be restored using FUNCTION RESTORE command
31) RESTORE <PAYLOAD> [FLUSH|APPEND|REPLACE]
32) Restore the libraries represented by the given payload, it is possible to give a restore policy to
33) control how to handle existing libraries (default APPEND):
34) * FLUSH: delete all existing libraries.
35) * APPEND: appends the restored libraries to the existing libraries. On collision, abort.
36) * REPLACE: appends the restored libraries to the existing libraries, On collision, replace the old
37) libraries with the new libraries (notice that even on this option there is a chance of failure
38) in case of functions name collision with another library).
39) HELP
40) Prints this help.
To reproduce
Steps to reproduce the behavior and/or a minimal code sample.
Expected behavior
A description of what you expected to happen.
Additional information
Any additional information that is relevant to the problem.
Comment From: oranagra
@leonchen83 thanks.
i see there's a contradiction between the documentation and the code.
the code expects description, but the docs mention desc.
usually when there's such a problem like wrong docs, i prefer to fix the docs, rather than the code (which someone may already rely on).
but since this is new, and "description" is too long, maybe we can do differently.
or maybe support both?
@MeirShpilraien @yossigo WDYT?
Comment From: yossigo
@oranagra The iron is still hot, let's use desc.
Comment From: MeirShpilraien
@yossigo @oranagra it was desc but changed due to review: https://github.com/redis/redis/pull/9780#discussion_r754829222
Comment From: yossigo
I'm OK either way, my point is I think this is NOT too late to change if we feel we have to.
Comment From: leonchen83
DESCRIPTION may better. DESC has different meaning in GEOSEARCHSTORE and SORT command
Comment From: oranagra
ok, so this was an intentional change in the code, and the docs (both in the json file, and also in the help message) are outdated. @MeirShpilraien can you make a PR to fix?
Comment From: MeirShpilraien
@oranagra sure
Comment From: leonchen83
BTW. could we support that add replace and description noop to the end of function load command? like RESTORE and COPY command
Comment From: oranagra
that's not possible. we need to decide where that mandatory is placed. it could either be a positional one like the serialized-value in RESTORE, or a last one like the timeout in BLPOP, can't have both.