Add dynamic_startup_nodes parameter to async RedisCluster #3447
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Kakadus
force-pushed
the
add-dynamic-startup-nodes-flag-to-async-redis-cluster
branch
from
c9ca010 to
cce34d7
Compare
Kakadus
force-pushed
the
add-dynamic-startup-nodes-flag-to-async-redis-cluster
branch
from
cce34d7 to
dd2ee3f
Compare
|
any news here? this looks good to me |
5 tasks
|
Hi @Kakadus would you have some time to complete this PR. There are some conflicts with the master branch that need to be resolved. |
There was a problem hiding this comment.
Pull Request Overview
This PR adds the "dynamic_startup_nodes" parameter to the async RedisCluster, aligning its behavior with the sync version.
- Introduces a new parameter to enable or disable dynamic DNS resolution for startup nodes.
- Implements handling in both the NodesManager and RedisCluster initialization.
- Adds parameterized tests to verify behavior with both True and False values.
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| tests/test_asyncio/test_cluster.py | Adds tests for dynamic startup nodes behavior using parameterized inputs |
| redis/asyncio/cluster.py | Updates RedisCluster and NodesManager to support the new dynamic_startup_nodes parameter |
* Fixed flacky TokenManager test * Fixed additional flacky test * Removed token count assertion * Skipped test on version 3.9
add CacheEntry Co-authored-by: zhousheng06 <[email protected]> Co-authored-by: Vladyslav Vildanov <[email protected]>
As of PEP 688, type checkers will no longer implicitly consider bytearray to be compatible with bytes
* Moved self._lock initialisation to Pool constructor * Added test case * Codestyle fixes * Added correct annotations
* Changed default dialect to 2 * Codestyle fixes * Fixed async tests * Added handling of RESP3 responses * Fixed flacky tests * Codestyle fix * Added separate file to hold default value
* Moved ClusterParser exceptions to BaseParser class * Codestyle fixes * Removed ubused imports * Sorted imports
Co-authored-by: Vladyslav Vildanov <[email protected]>
* DOC-4345 added testable JSON search examples for home page * DOC-4345 avoid possible non-deterministic results in tests * DOC-4345 close connection at end of example * DOC-4345 remove unnecessary blank lines
* Added Redis 8.0 to test matrix * Fixed test cases * Added version annotation * Changed FT.PROFILE response type * Added version restrictions * Updated file names, fixed tests assertions * Removed unused API
* Updated latest image to 8.0-M03-pre * Changed redis image for testing purposes * Updated image version * Updated redis server versions * Updated test case * Revert version restriction * Updated redis versions * Added tests for new default scorer * Skipped test on 8.0 * Fixed query to match exact-match syntax * Codestyle fixes * Added condition for 8.0-M04-pre image * Added test for INFO section
* Adding unit text fixes to improve compatibility with MacOS. * Applying review comments * Unifying the exception msg validation pattern for both test_connection.py files --------- Co-authored-by: Vladyslav Vildanov <[email protected]>
* fix: update redis university url * fix: add comment to changes --------- Co-authored-by: Vladyslav Vildanov <[email protected]>
…#3489) * Adding tests for modules ACL and modules config changes in 8.0 * Applying review comments * Adding deprecation annotations for tf config commands
Co-authored-by: Vladyslav Vildanov <[email protected]>
Co-authored-by: Vladyslav Vildanov <[email protected]>
Co-authored-by: Vladyslav Vildanov <[email protected]>
…: FT.INFO, just for async client: FT.CONFIG GET and FT.CONFIG SET (redis#3499)
* typing for client __init__ * typing with string literals * retry_on_error more specific typing * retry typing * fix lint --------- Co-authored-by: Vladyslav Vildanov <[email protected]>
… ft.info will not be deprecated in current release. (redis#3500)
* Marks old RediSearch 1.0 commands as deprecated * linters
…h ExponentialWithJitterBackoff become the default config. (redis#3614) * Changing the default retry configuration for Redis standalone clients. * Updating default retry strategy for standalone clients. 3 retries with ExponentialWithJitterBackoff become the default config. * Applying review comments - removing unused methods from retry objects, updating pydocs of error handler method
…initialized as part of error handling and retrying of the commands. (redis#3621)
…- the correct version is 5.3.0 (redis#3625)
… sync Cluster client to match sync/async cluster APIs (redis#3434) * Updated default value to much sync cluster API * Updated default value to TRUE --------- Co-authored-by: petyaslavova <[email protected]>
…#3622) * Adding default retry configuration changes for sync cluster client * Adding default retry configuration changes for sync cluster client * Adding default retry configuration changes for async cluster client * Updating docs related to retries and read_from_replicas. * Applying review comments. * Removing retry checks when using set_retry for cluster clients.
…security validations are not skipped by default (redis#3626) * Changing the default value for ssl_check_hostname to True, to ensure security validations are not skipped by default * Applying review comments * Removing unused operation in tests. * Removing unneeded comment from tests.
…ated with Optional - includes commands in core.py and json commands (redis#3610) * fix(redis-client): change `zrange` num parameter type to `Optional[int]` * fix(redis-client): normalize optional parameter annotations Replace all occurrences of Union[T, None] = None and bare T = None with Optional[T] = None in zrange, _zrange, arrtrim, and other methods so that type checkers no longer report errors. * commit message: fix(redis-client): normalize optional parameter annotations and correct arrtrim return type body: replaced all Union[T, None] = None and bare T = None with Optional[T] = None; changed arrtrim return annotation to Optional[int] * fix(redis-client): replace Optional[None] with Optional[int] for numeric parameters
Close redis#3630 Co-authored-by: Armin Berres <[email protected]>
…edis#3633) * Prevent RuntimeError while reinitializing clusters - sync and async * Applying copilot's review comments
* Test against unstable hiredis-py * Create a separate workflow instead * Remove outdated guard for hiredis-py The guard was required to prevent cluster tests on RESP3 with hiredis-py before 3.1.0 --------- Co-authored-by: petyaslavova <[email protected]>
* feat(cluster): support for transactions on cluster-aware client Adds support for transactions based on multi/watch/exec on clusters. Transactions in this mode are limited to a single hash slot. Contributed-by: Scopely <[email protected]> * fix: remove deprecated argument * remove attributions from code * Refactor ClusterPipeline to use execution strategies * Refactored strategy to use composition * Added test cases * Sync with master * Filter tests, ensure that tests are working after refactor * Added test case * Revert port changes * Improved exception handling * Change visibility of variable to public * Changed variable ref * Changed ref type * Added documentation * Refactored retries, fixed comments, fixed linters * Added word to a wordlist * Revert port changes * Added quotes * Fixed docs * Updated CONNECTION_ERRORS * Codestyle fixes * Updated docs * Revert import --------- Co-authored-by: vladvildanov <[email protected]> Co-authored-by: Vladyslav Vildanov <[email protected]>
…erification with ssl_cert_reqs="none", check_hostname is set to False (redis#3637) * Fix SSL verification with ssl_cert_reqs=none and ssl_check_hostname=True * Add ssl_check_hostname to REDIS_ALLOWED_KEYS and fix default value in RedisSSLContext
…atest stable release (redis#3644)
5 tasks
|
Hi @Kakadus, |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Pull Request check-list
Description of change
The async version of the RedisCluster lacked the
dynamic_startup_nodesparameter. This PR adds the parameter to be activated when dynamic DNS endpoints for startup nodes are in use.Enabling
dynamic_startup_nodesfixes #2472, which describes the problem in detail.#3111 also addresses said issue, but their logic is different to the sync version, while this approach moves the behaviour of the async version closer to the sync version.