search

aws / glide-for-redis

Apache License 2.0
153 stars 29 forks source link

Java: Add `BZMPOP` command #1390

Closed Yury-Fridlyand closed 1 month ago

Yury-Fridlyand commented 1 month ago

Issue #, if available: N/A

Description of changes: https://redis.io/commands/bzmpop/

Ref: #1255 and #1248

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

aaron-congo commented 1 month ago

I implemented this for Python (PR is not up yet) and the transaction tests were failing with a cross slot error. This was because BZMPOP needs to be added to cluster_routing.rs#base_routing, since it doesn't fall under the default FirstKey category. Mapping BZMPOP to ThirdArgAfterKeyCount locally solved the problem for me. Do you know why the transaction tests are passing here even though cluster_routing.rs hasn't been updated for BZMPOP yet?

Yury-Fridlyand commented 1 month ago

I implemented this for Python (PR is not up yet) and the transaction tests were failing with a cross slot error. This was because BZMPOP needs to be added to cluster_routing.rs#base_routing, since it doesn't fall under the default FirstKey category. Mapping BZMPOP to ThirdArgAfterKeyCount locally solved the problem for me. Do you know why the transaction tests are passing here even though cluster_routing.rs hasn't been updated for BZMPOP yet?

No idea. I know, few tests in #1284 fail which pass here. Looking forward to run tests with fixed routing.