DiceDB / dice

DiceDB is a redis-compliant, reactive, scalable, highly-available, unified cache optimized for modern hardware.
https://dicedb.io/
Other
6.88k stars 1.09k forks source link

#1031: Bloom Filter - Migrated `bf.add`, `bf.reserve`, `bf.exists` & `bf.info` to store_eval #1117

Closed apoorvyadav1111 closed 1 month ago

apoorvyadav1111 commented 1 month ago

Fixes: #1031 #1110

For migration commands, please consider this a template on how to tackle the issue. I have divided the tasks into smaller goals and made an individual commit. Condition Return Value
Migrating eval to store eval 7883d48
Modify unit test with new response format 93c2358
Migrate integration tests from async to resp b33cd1d
Add/update integration in websocket /97744b6
Add/update integration in HTTP 3a251d8
Ensure all integration test pass 3a251d8
Update/Add docs for the commands 66dd05e
Lint checks 32216df

Checklist

AshwinKul28 commented 1 month ago

Thanks for this PR @apoorvyadav1111 . Fantastic effort. Everything looks good and we can now reuse evalError and evalResponse functions in all store evals. We may not ask others to do the changes again if other PRs are already in the last stage, we will do it by ourselves once all commands are migrated.

Also, I see your commit links are broken in the PR description for now, once it's added to master I believe it should work.

Thanks again. I just have one comment, post resolution of that feel free to merge.

apoorvyadav1111 commented 1 month ago

Hi @AshwinKul28 , thanks for the review. It must be due to force-push, for the safer side, I will update the commits once the branch gets merged and will not be modified.

apoorvyadav1111 commented 1 month ago

Hi @AshwinKul28 , Apologies I merged it before looking at the new commits, I can definitely raise a new PR right now and add those comments. I have updated the commit hashes for now.