-
Notifications
You must be signed in to change notification settings - Fork 262
Add command info registration #885
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
…TopK; update Makefile and rebloom.c
…e rebloom.c to include BF command registration.
… CF.RESERVE and BF.ADD in Redis module; introduce new utility functions for documentation testing.
…fo command (#892) * MOD-10854 BF.EXISTS info command * add test * clang * MOD-10855 add also bf.info info * MOD-10853 add also bf.card info * MOD-10856 add also bf.insert info * MOD-10857 add also bf.insert info * MOD-10858 add also bf.madd info + arity fix * MOD-10859 add also bf.mexists * MOD-10860 add also bf.reserve * MOD-10861 add also bf.scandump * fix tests * fix * fix build * fix keyspecs range for all commands * correct keystep * correct keystep
* MOD-10866 cf.del info * MOD-10867 cf.exists info * MOD-10868 cf.info info * MOD-10869 cf.insert info * MOD-10870 cf.insertnx info * MOD-10871 cf.locadchunk info * MOD-10872 cf.mexists info * MOD-10874 cf.scandump info * fix tests
* MOD-10854 BF.EXISTS info command * add test * clang * MOD-10855 add also bf.info info * MOD-10853 add also bf.card info * MOD-10856 add also bf.insert info * MOD-10857 add also bf.insert info * MOD-10858 add also bf.madd info + arity fix * MOD-10859 add also bf.mexists * MOD-10860 add also bf.reserve * MOD-10861 add also bf.scandump * fix tests * fix * fix build * fix keyspecs range for all commands * correct keystep * correct keystep * MOD-10881 add topk.add info * MOD-10882 add topk.count info * MOD-10883 add topk.incrby info * MOD-10884 add topk.info info * MOD-10885 add topk.list info * MOD-10886 add topk.query info * MOD-10887 add topk.reserve info * MOD-10875 add cms.incrby info * MOD-10876 add cms.info info * MOD-10877 add cms.initbydim info * MOD-10878 add cms.initbyprob info * MOD-10879 add cms.merge info * MOD-10880 add cms.query info * indentation
… the info in commands.json
… the info in commands.json
…h the info in commands.json
…th the info in commands.json
src/cmd_info/cf_info.c
Outdated
| .flags = REDISMODULE_CMD_ARG_OPTIONAL, | ||
| .subargs = | ||
| (RedisModuleCommandArg[]){ | ||
| {.name = "capacity", .type = REDISMODULE_ARG_TYPE_PURE_TOKEN, .token = "CAPACITY"}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The subargs for capacity should be (RedisModuleCommandArg[]){{.name = "capacity", .type = REDISMODULE_ARG_TYPE_INTEGER}, {0}}} and the .token field on the capacity block argument should be set to "CAPACITY".
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
CF.INSERTNX key [CAPACITY capacity] [NOCREATE] ITEMS item [item ...]
src/cmd_info/cf_info.c
Outdated
| {.name = "bucketsize", | ||
| .type = REDISMODULE_ARG_TYPE_BLOCK, | ||
| .flags = REDISMODULE_CMD_ARG_OPTIONAL, | ||
| .subargs = (RedisModuleCommandArg[]){{.name = "CAPACITY", .type = REDISMODULE_ARG_TYPE_STRING}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Each optional argument block should have a token (e.g., "BUCKETSIZE") and its subargs should be (RedisModuleCommandArg[]){{.name = "bucketsize", .type = REDISMODULE_ARG_TYPE_INTEGER}, {0}}}.
src/cmd_info/tdigest_info.c
Outdated
| // [OVERRIDE] | ||
| // =============================== | ||
| static const RedisModuleCommandKeySpec TDIGEST_MERGE_KEYSPECS[] = { | ||
| {.notes = "", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should define 2 key pec to this commands, one for dst key and the second for the source keys
src/cmd_info/topk_info.c
Outdated
| return REDISMODULE_ERR; | ||
| } | ||
| if (RedisModule_SetCommandInfo(cmd_list, &TOPK_LIST_INFO) == REDISMODULE_ERR) { | ||
| printf("TOPK.LIST command info set error\n"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
remove
| @@ -0,0 +1,700 @@ | |||
| from docs_utils import * | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tests for CF.RESERVE, CF.INSERT, CF.INSERTNX, and TDIGEST.MERGE are missing.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
?!
They are all there.
… 'notes' field for commands info
This pull request introduces a centralized mechanism for registering command metadata (COMMAND INFO) for RedisBloom's data structures. The primary goal is to provide detailed information about commands—including their syntax, arguments, complexity, and other metadata—which is exposed via the Redis COMMAND command. This change improves introspection and discoverability of the module's commands. The engineering approach involves creating separate .c files for each data type (Bloom Filter, Cuckoo Filter, etc.) to define the command information structs and then registering them during module initialization, which is a clean and scalable approach. A comprehensive test suite has been added to verify the correctness of the command documentation.