-
Notifications
You must be signed in to change notification settings - Fork 54
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
Java: Add binary support to custom command. #2109
Java: Add binary support to custom command. #2109
Conversation
Signed-off-by: Yury-Fridlyand <[email protected]>
* | ||
* <p>The command will be routed to all primaries. | ||
* subcommands, should be added as a separate value in <code>args</code>.<br> | ||
* The command will be routed automatically based on the passed command's default request policy. |
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.
Is this correct?
What about commands that don't have a default request policy?
Users may ask where to find this default request policy... which we can't provide except through the code.
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.
I guess we should update https:/valkey-io/valkey-glide/wiki/General-Concepts#custom-command
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.
Yes, we should.
I copied this line from python/node docs.
Policy is defined in redis-rs
in cluster_routing.rs
java/client/src/main/java/glide/api/commands/GenericClusterCommands.java
Outdated
Show resolved
Hide resolved
Co-authored-by: Andrew Carbonetto <[email protected]> Signed-off-by: Yury-Fridlyand <[email protected]>
Signed-off-by: Yury-Fridlyand <[email protected]>
Co-authored-by: Guian Gumpac <[email protected]> Signed-off-by: Yury-Fridlyand <[email protected]>
…06-custom-bin Signed-off-by: Yury-Fridlyand <[email protected]>
…ll/glide-for-redis into java/yuryf-valkey-206-custom-bin Signed-off-by: Yury-Fridlyand <[email protected]>
Signed-off-by: Yury-Fridlyand <[email protected]>
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.
Most of this looks fine, but Andrew's comment on the TODO should be addressed.
Signed-off-by: Andrew Carbonetto <[email protected]>
And update docs for custom command in python and node clients.
Isn't a breaking change, because
ReturnType
/TResult
already includes a mapping from string toReturnType
/TResult
, which is equviavalent toClusterResponse<ReturnType>
/TClusterResponse[TResult]
.