-
Notifications
You must be signed in to change notification settings - Fork 262
BF.CARD/EXISTS/INFO/INSERT/LOADCHUNK/MADD/MEXISTS/RESERVE/SCANDUMP info command #892
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
BF.CARD/EXISTS/INFO/INSERT/LOADCHUNK/MADD/MEXISTS/RESERVE/SCANDUMP info command #892
Conversation
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.
Pull Request Overview
This PR adds comprehensive command information documentation for 9 Bloom filter commands (BF.CARD, BF.EXISTS, BF.INFO, BF.INSERT, BF.LOADCHUNK, BF.MADD, BF.MEXISTS, BF.RESERVE, BF.SCANDUMP). The changes include both C command definitions and corresponding Python tests.
Key changes:
- Added complete command info structures for 9 Bloom filter commands in C
- Added corresponding test cases to validate the command documentation
- Fixed test utilities to properly handle argument validation
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| tests/flow/test_docs_help.py | Added 9 new test methods to validate command documentation for each Bloom filter command |
| tests/flow/test_acl.py | Fixed ACL test commands to include required arguments |
| tests/flow/docs_utils.py | Fixed argument name extraction to handle tuple structure correctly |
| src/cmd_info/bf_info.c | Added complete command info definitions and registration for 9 Bloom filter commands |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| if args is not None: | ||
| norm_args = [_kv_list_to_dict(a) for a in (docs.get('arguments') or docs.get('args') or [])] | ||
| env.assertEqual([a.get('name') for a in norm_args], [n for n, _t in args]) | ||
| env.assertEqual([a.get('name') for a in norm_args], [n[0] for n in args]) |
Copilot
AI
Sep 3, 2025
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 change from [n for n, _t in args] to [n[0] for n in args] assumes args elements are tuples/lists, but this could fail if args contains simple strings. The original tuple unpacking was safer as it handled the expected tuple structure explicitly.
| env.assertEqual([a.get('name') for a in norm_args], [n[0] for n in args]) | |
| env.assertEqual([a.get('name') for a in norm_args], [n for n, _t in args]) |
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.
args is a tuple that may contain more tha 2 elements, thats why im doing [0]
* Add command info registration for Bloom Filters, Cuckoo Filters, and TopK; update Makefile and rebloom.c * format * Add registration for Bloom filter command info in Redis module; update rebloom.c to include BF command registration. * Add TDIGEST command info registration and update command metadata for CF.RESERVE and BF.ADD in Redis module; introduce new utility functions for documentation testing. * BF.CARD/EXISTS/INFO/INSERT/LOADCHUNK/MADD/MEXISTS/RESERVE/SCANDUMP info 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 * Add more CF info commands (#897) * 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 * Topk and CMS info commands (#896) * 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 * lwoercase arg name * Added parameters specs to each command's comment * Added some missing CF commands * MOD-10888 tdigest.add info * MOD-10889 tdigest.byrank info * MOD-10890 tdigest.byrevrank info * MOD-10891 tdigest.cdf info * MOD-10892 tdigest.create info * MOD-10893 tdigest.max info * clang * Align 'summary', 'complexity' and 'since' fields of CF. commands with the info in commands.json * Align 'summary', 'complexity' and 'since' fields of BF. commands with the info in commands.json * Align 'summary', 'complexity' and 'since' fields of CMS. commands with the info in commands.json * Align 'summary', 'complexity' and 'since' fields of TOPK. commands with the info in commands.json * Most 'notes' were rubbish * Completed info on tdigest commands * Fixed failing tests * format the code and hush the linter * Added missing tests + some info fixes * Fixed the CAPACITY param of CF.INSERTNX * Fixed parameters of CF.RESERVE * Added cmd info on source-key of TDIGEST.MERGE * cleanup * cmd info for tdigest.merge fixed * Fixed test * Removed all notes, since they don't belong to the key and there is no 'notes' field for commands info --------- Co-authored-by: Aviv David <40210928+AvivDavid23@users.noreply.github.com> Co-authored-by: avivdavid23 <avivdavid2312@gmail.com> Co-authored-by: Gal Cohen <gal@redis.com>
Uh oh!
There was an error while loading. Please reload this page.