Align empty cluster target handling - #4324
GruffElixir wants to merge 1 commit into
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4fe681374d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| target_nodes = None | ||
| passed_targets = kwargs.pop("target_nodes", None) | ||
| if passed_targets is not None and not self._is_nodes_flag(passed_targets): | ||
| if passed_targets and not self._is_nodes_flag(passed_targets): |
There was a problem hiding this comment.
Reject unsupported false-y target types
When target_nodes is False, 0, (), or b"", this condition now skips _parse_target_nodes; _determine_nodes also ignores the false-y node flag, so the command is dispatched using normal routing. Before this commit these unsupported types raised TypeError, which matters especially for writes because an invalid caller or configuration value now executes rather than failing fast. Limit the fallback to the deliberately accepted empty list, mapping, and string instead of all false-y objects, and mirror that validation in async.
AGENTS.md reference: AGENTS.md:L159-L163
Useful? React with 👍 / 👎.
What changed
The synchronous cluster client now treats empty
target_nodesvalues the same way as the async client and pipeline implementation: they fall back to normal command-policy routing instead of being treated as an explicit empty target set.Regression tests cover empty lists, mappings, and strings for both sync and async clients.
Fixes #4318.
Validation
uv run --no-project --with pytest --with pytest-asyncio --with async-timeout python -m pytest tests/test_cluster.py tests/test_asyncio/test_cluster.py -q -k empty_target_nodesuv run --no-project --with ruff ruff check redis/cluster.py tests/test_cluster.py tests/test_asyncio/test_cluster.pygit diff --checkNote
Low Risk
Single conditional change in cluster command routing with targeted regression tests; behavior aligns sync with existing async/pipeline semantics.
Overview
Fixes cluster routing when
target_nodesis empty. The syncRedisCluster.execute_commandpath now usesif passed_targetsinstead ofif passed_targets is not None, matching the async client and pipeline behavior.Empty values (
[],{},"") are no longer treated as an explicit (empty) target set; commands fall back to normal policy routing and run on the default node.Regression tests for sync and async assert
DBSIZEwith those emptytarget_nodesvalues delegates to_execute_commandon the default node only.Fixes #4318.
Reviewed by Cursor Bugbot for commit 4fe6813. Bugbot is set up for automated code reviews on this repo. Configure here.