Fix custom command response handling #3593

Yury-Fridlyand · 2025-04-12T02:08:55Z

Fix response handling for custom command on cluster client
Tests
spotless

Signed-off-by: Yury-Fridlyand <[email protected]>

tjzhang-BQ · 2025-04-15T22:21:40Z

what was the context of this again? looks similar to the functions command behaviors

Yury-Fridlyand · 2025-04-15T22:41:22Z

Custom command didn't work properly with commands which have a response aggregation policy and with commands which return a map from a single node (e.g. config get).
For example, dbsize have sent to all nodes still returns a single number.

edlng

lgtm!

* Fix custom command response handling Signed-off-by: Yury-Fridlyand <[email protected]> Signed-off-by: jbrinkman <[email protected]>

* Refactor: Update client creation methods to use named clients for better clarity and consistency in PubSub tests Signed-off-by: jbrinkman <[email protected]> * Close Client Safety Signed-off-by: jbrinkman <[email protected]> * Add Cluster support, refactor PubSub tests, and remove deprecated files Signed-off-by: jbrinkman <[email protected]> * remove redundant func Signed-off-by: jbrinkman <[email protected]> * review feedback Signed-off-by: jbrinkman <[email protected]> * Add PubSubHandler interface and GetQueue method to BaseClient; enhance PubSubMessageQueue with signal channel support; refactor integration tests to utilize new client creation methods and improve message handling. Signed-off-by: jbrinkman <[email protected]> * Refactor PubSub integration tests to consolidate message receipt patterns into a single comprehensive test function, improving maintainability and readability. This change introduces a parameterized approach to test various client types and message reading methods. Signed-off-by: jbrinkman <[email protected]> * Refactor and enhance PubSub integration tests by consolidating message verification logic and introducing parameterized tests for various client types and message reading methods. This update improves test maintainability and readability while ensuring comprehensive coverage of PubSub functionality. Signed-off-by: jbrinkman <[email protected]> * Remove outdated PubSub integration test file pubsub_handler_test.go to streamline the test suite and eliminate redundancy. This change helps maintain focus on current testing strategies and improves overall test suite organization. Signed-off-by: jbrinkman <[email protected]> * Improve error handling in close_client function by logging a warning when the client adapter pointer is not null. Remove unused PubSub test functions to streamline the test suite and enhance readability. Signed-off-by: jbrinkman <[email protected]> * C#: benchmark fix (#3612) benchmark fix Signed-off-by: Yury-Fridlyand <[email protected]> Signed-off-by: jbrinkman <[email protected]> * Docs: Updates READMEs with docs site (#3616) * apdates readmes with docs site Signed-off-by: Lior Sventitzky <[email protected]> * Update node/README.md Co-authored-by: Avi Fenesh <[email protected]> Signed-off-by: Lior Sventitzky <[email protected]> * Update python/README.md Co-authored-by: Avi Fenesh <[email protected]> Signed-off-by: Lior Sventitzky <[email protected]> * moved docs up Signed-off-by: Lior Sventitzky <[email protected]> --------- Signed-off-by: Lior Sventitzky <[email protected]> Co-authored-by: Avi Fenesh <[email protected]> Signed-off-by: jbrinkman <[email protected]> * Core: Move UDS Socket Filename to tmp (#3615) * Fix: Move socket name to tmp/ Signed-off-by: BoazBD <[email protected]> * add to changelog Signed-off-by: BoazBD <[email protected]> * fix documentation and add macos address in use string Signed-off-by: BoazBD <[email protected]> * lint Signed-off-by: BoazBD <[email protected]> * make if and else return &Path Signed-off-by: BoazBD <[email protected]> * remove folder type Signed-off-by: BoazBD <[email protected]> --------- Signed-off-by: BoazBD <[email protected]> Signed-off-by: jbrinkman <[email protected]> * Java: Re-enable `test_update_connection_password` (#3623) Signed-off-by: jbrinkman <[email protected]> * C#: Compile on C# 10 (.net 6) (#3412) * Compile on C# 10 (.net 6) --------- Signed-off-by: Yury-Fridlyand <[email protected]> Signed-off-by: jbrinkman <[email protected]> * Go: Add command FUNCTION KILL (#3604) Signed-off-by: TJ Zhang <[email protected]> Signed-off-by: jbrinkman <[email protected]> * Updated attribution files for commit 6d0731a (#3591) Updated attribution files Signed-off-by: ort-bot <[email protected]> Co-authored-by: ort-bot <[email protected]> Signed-off-by: jbrinkman <[email protected]> * Address PR feedback Signed-off-by: jbrinkman <[email protected]> * Rust: Fix rust bench latencies calculation (#3636) Signed-off-by: barshaul <[email protected]> Signed-off-by: jbrinkman <[email protected]> * Go: Implement BZPOPMAX and ZMPOP (#3257) Signed-off-by: MikeMwita <[email protected]> Signed-off-by: jbrinkman <[email protected]> * Fix custom command response handling (#3593) * Fix custom command response handling Signed-off-by: Yury-Fridlyand <[email protected]> Signed-off-by: jbrinkman <[email protected]> * Updated attribution files for commit 0d89fdd (#3640) Updated attribution files Signed-off-by: ort-bot <[email protected]> Co-authored-by: ort-bot <[email protected]> Signed-off-by: jbrinkman <[email protected]> * C#: Error handling. (#3411) * Error handling. Signed-off-by: Yury-Fridlyand <[email protected]> Signed-off-by: jbrinkman <[email protected]> * Create stale.yml (#3638) * Create stale.yml To control accumulating issues and PRS, issues with no activity for 90 days, which are not a bug, user issue, or part of a milestone, will be marked as stale. Same as for PR but for 60 days. To unstaled, any activity is sufficient, like comment or anything similar. If after staling, the issue will not see any activity for 14 days, and PR for 10, it will be closed automatically. To not erase a work of a dev that missed the notification, the branch won't be deleted and the PR/issue can be re-opened. Signed-off-by: Avi Fenesh <[email protected]> * Update stale.yml Signed-off-by: Avi Fenesh <[email protected]> * Update stale.yml Signed-off-by: Avi Fenesh <[email protected]> * indent Signed-off-by: Avi Fenesh <[email protected]> --------- Signed-off-by: Avi Fenesh <[email protected]> Signed-off-by: jbrinkman <[email protected]> * Core/Go: Added Ok response type to FFI, updated Go with Ok handlers (#3630) * Added Ok response type to ffi, update go accordingly Signed-off-by: Lior Sventitzky <[email protected]> * addressed comments Signed-off-by: Lior Sventitzky <[email protected]> * added CHANGELOG entry Signed-off-by: Lior Sventitzky <[email protected]> * changed updateConnectionPassword and RestoreWithOptions to return string Signed-off-by: Lior Sventitzky <[email protected]> --------- Signed-off-by: Lior Sventitzky <[email protected]> Signed-off-by: jbrinkman <[email protected]> * fix linting errors due to unused variable Signed-off-by: jbrinkman <[email protected]> * Go: Add command FUNCTION KILL (#3604) Signed-off-by: TJ Zhang <[email protected]> Signed-off-by: jbrinkman <[email protected]> * Core/Go: Added Ok response type to FFI, updated Go with Ok handlers (#3630) * Added Ok response type to ffi, update go accordingly Signed-off-by: Lior Sventitzky <[email protected]> * addressed comments Signed-off-by: Lior Sventitzky <[email protected]> * added CHANGELOG entry Signed-off-by: Lior Sventitzky <[email protected]> * changed updateConnectionPassword and RestoreWithOptions to return string Signed-off-by: Lior Sventitzky <[email protected]> --------- Signed-off-by: Lior Sventitzky <[email protected]> Signed-off-by: jbrinkman <[email protected]> * Enhance client closure handling in FFI by checking strong reference count. Added warning for remaining references to the client adapter upon closure to prevent potential resource leaks. Signed-off-by: jbrinkman <[email protected]> --------- Signed-off-by: jbrinkman <[email protected]> Signed-off-by: Yury-Fridlyand <[email protected]> Signed-off-by: Lior Sventitzky <[email protected]> Signed-off-by: BoazBD <[email protected]> Signed-off-by: TJ Zhang <[email protected]> Signed-off-by: ort-bot <[email protected]> Signed-off-by: barshaul <[email protected]> Signed-off-by: MikeMwita <[email protected]> Signed-off-by: Avi Fenesh <[email protected]> Co-authored-by: Yury-Fridlyand <[email protected]> Co-authored-by: Lior Sventitzky <[email protected]> Co-authored-by: Avi Fenesh <[email protected]> Co-authored-by: BoazBD <[email protected]> Co-authored-by: Edward Liang <[email protected]> Co-authored-by: tjzhang-BQ <[email protected]> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: ort-bot <[email protected]> Co-authored-by: Bar Shaul <[email protected]> Co-authored-by: Mike Mwita <[email protected]>

* Fix custom command response handling Signed-off-by: Yury-Fridlyand <[email protected]>

* Refactor: Update client creation methods to use named clients for better clarity and consistency in PubSub tests Signed-off-by: jbrinkman <[email protected]> * Close Client Safety Signed-off-by: jbrinkman <[email protected]> * Add Cluster support, refactor PubSub tests, and remove deprecated files Signed-off-by: jbrinkman <[email protected]> * remove redundant func Signed-off-by: jbrinkman <[email protected]> * review feedback Signed-off-by: jbrinkman <[email protected]> * Add PubSubHandler interface and GetQueue method to BaseClient; enhance PubSubMessageQueue with signal channel support; refactor integration tests to utilize new client creation methods and improve message handling. Signed-off-by: jbrinkman <[email protected]> * Refactor PubSub integration tests to consolidate message receipt patterns into a single comprehensive test function, improving maintainability and readability. This change introduces a parameterized approach to test various client types and message reading methods. Signed-off-by: jbrinkman <[email protected]> * Refactor and enhance PubSub integration tests by consolidating message verification logic and introducing parameterized tests for various client types and message reading methods. This update improves test maintainability and readability while ensuring comprehensive coverage of PubSub functionality. Signed-off-by: jbrinkman <[email protected]> * Remove outdated PubSub integration test file pubsub_handler_test.go to streamline the test suite and eliminate redundancy. This change helps maintain focus on current testing strategies and improves overall test suite organization. Signed-off-by: jbrinkman <[email protected]> * Improve error handling in close_client function by logging a warning when the client adapter pointer is not null. Remove unused PubSub test functions to streamline the test suite and enhance readability. Signed-off-by: jbrinkman <[email protected]> * C#: benchmark fix (#3612) benchmark fix Signed-off-by: Yury-Fridlyand <[email protected]> Signed-off-by: jbrinkman <[email protected]> * Docs: Updates READMEs with docs site (#3616) * apdates readmes with docs site Signed-off-by: Lior Sventitzky <[email protected]> * Update node/README.md Co-authored-by: Avi Fenesh <[email protected]> Signed-off-by: Lior Sventitzky <[email protected]> * Update python/README.md Co-authored-by: Avi Fenesh <[email protected]> Signed-off-by: Lior Sventitzky <[email protected]> * moved docs up Signed-off-by: Lior Sventitzky <[email protected]> --------- Signed-off-by: Lior Sventitzky <[email protected]> Co-authored-by: Avi Fenesh <[email protected]> Signed-off-by: jbrinkman <[email protected]> * Core: Move UDS Socket Filename to tmp (#3615) * Fix: Move socket name to tmp/ Signed-off-by: BoazBD <[email protected]> * add to changelog Signed-off-by: BoazBD <[email protected]> * fix documentation and add macos address in use string Signed-off-by: BoazBD <[email protected]> * lint Signed-off-by: BoazBD <[email protected]> * make if and else return &Path Signed-off-by: BoazBD <[email protected]> * remove folder type Signed-off-by: BoazBD <[email protected]> --------- Signed-off-by: BoazBD <[email protected]> Signed-off-by: jbrinkman <[email protected]> * Java: Re-enable `test_update_connection_password` (#3623) Signed-off-by: jbrinkman <[email protected]> * C#: Compile on C# 10 (.net 6) (#3412) * Compile on C# 10 (.net 6) --------- Signed-off-by: Yury-Fridlyand <[email protected]> Signed-off-by: jbrinkman <[email protected]> * Go: Add command FUNCTION KILL (#3604) Signed-off-by: TJ Zhang <[email protected]> Signed-off-by: jbrinkman <[email protected]> * Updated attribution files for commit 6d0731a (#3591) Updated attribution files Signed-off-by: ort-bot <[email protected]> Co-authored-by: ort-bot <[email protected]> Signed-off-by: jbrinkman <[email protected]> * Address PR feedback Signed-off-by: jbrinkman <[email protected]> * Rust: Fix rust bench latencies calculation (#3636) Signed-off-by: barshaul <[email protected]> Signed-off-by: jbrinkman <[email protected]> * Go: Implement BZPOPMAX and ZMPOP (#3257) Signed-off-by: MikeMwita <[email protected]> Signed-off-by: jbrinkman <[email protected]> * Fix custom command response handling (#3593) * Fix custom command response handling Signed-off-by: Yury-Fridlyand <[email protected]> Signed-off-by: jbrinkman <[email protected]> * Updated attribution files for commit 0d89fdd (#3640) Updated attribution files Signed-off-by: ort-bot <[email protected]> Co-authored-by: ort-bot <[email protected]> Signed-off-by: jbrinkman <[email protected]> * C#: Error handling. (#3411) * Error handling. Signed-off-by: Yury-Fridlyand <[email protected]> Signed-off-by: jbrinkman <[email protected]> * Create stale.yml (#3638) * Create stale.yml To control accumulating issues and PRS, issues with no activity for 90 days, which are not a bug, user issue, or part of a milestone, will be marked as stale. Same as for PR but for 60 days. To unstaled, any activity is sufficient, like comment or anything similar. If after staling, the issue will not see any activity for 14 days, and PR for 10, it will be closed automatically. To not erase a work of a dev that missed the notification, the branch won't be deleted and the PR/issue can be re-opened. Signed-off-by: Avi Fenesh <[email protected]> * Update stale.yml Signed-off-by: Avi Fenesh <[email protected]> * Update stale.yml Signed-off-by: Avi Fenesh <[email protected]> * indent Signed-off-by: Avi Fenesh <[email protected]> --------- Signed-off-by: Avi Fenesh <[email protected]> Signed-off-by: jbrinkman <[email protected]> * Core/Go: Added Ok response type to FFI, updated Go with Ok handlers (#3630) * Added Ok response type to ffi, update go accordingly Signed-off-by: Lior Sventitzky <[email protected]> * addressed comments Signed-off-by: Lior Sventitzky <[email protected]> * added CHANGELOG entry Signed-off-by: Lior Sventitzky <[email protected]> * changed updateConnectionPassword and RestoreWithOptions to return string Signed-off-by: Lior Sventitzky <[email protected]> --------- Signed-off-by: Lior Sventitzky <[email protected]> Signed-off-by: jbrinkman <[email protected]> * fix linting errors due to unused variable Signed-off-by: jbrinkman <[email protected]> * Go: Add command FUNCTION KILL (#3604) Signed-off-by: TJ Zhang <[email protected]> Signed-off-by: jbrinkman <[email protected]> * Core/Go: Added Ok response type to FFI, updated Go with Ok handlers (#3630) * Added Ok response type to ffi, update go accordingly Signed-off-by: Lior Sventitzky <[email protected]> * addressed comments Signed-off-by: Lior Sventitzky <[email protected]> * added CHANGELOG entry Signed-off-by: Lior Sventitzky <[email protected]> * changed updateConnectionPassword and RestoreWithOptions to return string Signed-off-by: Lior Sventitzky <[email protected]> --------- Signed-off-by: Lior Sventitzky <[email protected]> Signed-off-by: jbrinkman <[email protected]> * Enhance client closure handling in FFI by checking strong reference count. Added warning for remaining references to the client adapter upon closure to prevent potential resource leaks. Signed-off-by: jbrinkman <[email protected]> --------- Signed-off-by: jbrinkman <[email protected]> Signed-off-by: Yury-Fridlyand <[email protected]> Signed-off-by: Lior Sventitzky <[email protected]> Signed-off-by: BoazBD <[email protected]> Signed-off-by: TJ Zhang <[email protected]> Signed-off-by: ort-bot <[email protected]> Signed-off-by: barshaul <[email protected]> Signed-off-by: MikeMwita <[email protected]> Signed-off-by: Avi Fenesh <[email protected]> Co-authored-by: Yury-Fridlyand <[email protected]> Co-authored-by: Lior Sventitzky <[email protected]> Co-authored-by: Avi Fenesh <[email protected]> Co-authored-by: BoazBD <[email protected]> Co-authored-by: Edward Liang <[email protected]> Co-authored-by: tjzhang-BQ <[email protected]> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: ort-bot <[email protected]> Co-authored-by: Bar Shaul <[email protected]> Co-authored-by: Mike Mwita <[email protected]>

Yury-Fridlyand added 2 commits April 11, 2025 19:05

Fix custom command response handling

667bde3

Signed-off-by: Yury-Fridlyand <[email protected]>

changelong

72c15bc

Signed-off-by: Yury-Fridlyand <[email protected]>

Yury-Fridlyand added java issues and fixes related to the java client go golang wrapper labels Apr 12, 2025

Yury-Fridlyand requested a review from a team as a code owner April 12, 2025 02:08

Yury-Fridlyand added 2 commits April 11, 2025 19:55

Typo fix.

e8f7ad1

Signed-off-by: Yury-Fridlyand <[email protected]>

Typo fix.

a06cd47

Signed-off-by: Yury-Fridlyand <[email protected]>

yipin-chen approved these changes Apr 15, 2025

View reviewed changes

edlng approved these changes Apr 22, 2025

View reviewed changes

Yury-Fridlyand merged commit 0d89fdd into valkey-io:main Apr 23, 2025
25 checks passed

Yury-Fridlyand deleted the java/yuryf-fix-custom-command branch April 23, 2025 00:25

jbrinkman pushed a commit that referenced this pull request Apr 23, 2025

Fix custom command response handling (#3593)

afe922a

* Fix custom command response handling Signed-off-by: Yury-Fridlyand <[email protected]> Signed-off-by: jbrinkman <[email protected]>

jbrinkman pushed a commit that referenced this pull request Apr 23, 2025

Fix custom command response handling (#3593)

ba403a8

* Fix custom command response handling Signed-off-by: Yury-Fridlyand <[email protected]> Signed-off-by: jbrinkman <[email protected]>

jbrinkman pushed a commit that referenced this pull request Apr 23, 2025

Fix custom command response handling (#3593)

896ffe9

* Fix custom command response handling Signed-off-by: Yury-Fridlyand <[email protected]> Signed-off-by: jbrinkman <[email protected]>

ikolomi pushed a commit that referenced this pull request May 11, 2025

Fix custom command response handling (#3593)

261469d

* Fix custom command response handling Signed-off-by: Yury-Fridlyand <[email protected]>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Fix custom command response handling #3593

Fix custom command response handling #3593

Uh oh!

Yury-Fridlyand commented Apr 12, 2025

Uh oh!

tjzhang-BQ commented Apr 15, 2025

Uh oh!

Yury-Fridlyand commented Apr 15, 2025 •

edited

Loading

Uh oh!

edlng left a comment

Uh oh!

Uh oh!

Uh oh!

Fix custom command response handling #3593

Fix custom command response handling #3593

Uh oh!

Conversation

Yury-Fridlyand commented Apr 12, 2025

Uh oh!

tjzhang-BQ commented Apr 15, 2025

Uh oh!

Yury-Fridlyand commented Apr 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

edlng left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Yury-Fridlyand commented Apr 15, 2025 •

edited

Loading