-
Notifications
You must be signed in to change notification settings - Fork 2.6k
Added async implementation of MultiDBClient #3762
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
base: feat/active-active
Are you sure you want to change the base?
Conversation
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.
Pull Request Overview
This PR adds async implementation of MultiDBClient to support asynchronous operations with multiple Redis databases. The implementation provides circuit breaker patterns, health checks, and failure detection in an async context.
- Replaces
SyncCircuitBreaker
with unifiedCircuitBreaker
interface across all components - Adds complete async multidb module with client, configuration, command execution, and health checks
- Extends background scheduler with async recurring task support
Reviewed Changes
Copilot reviewed 29 out of 32 changed files in this pull request and generated 4 comments.
Show a summary per file
File | Description |
---|---|
tests/test_multidb/test_config.py | Updates circuit breaker imports and removes empty lines |
tests/test_multidb/test_client.py | Removes unnecessary blank lines in test assertions |
tests/test_multidb/test_circuit.py | Updates circuit breaker type references in imports and callbacks |
tests/test_multidb/conftest.py | Updates all mock circuit breaker references to unified type |
tests/test_background.py | Adds async test coverage for recurring background tasks |
tests/test_asyncio/ | Comprehensive test suite for async multidb components |
redis/utils.py | Adds async dummy failure function for retry operations |
redis/multidb/ | Updates circuit breaker interface and removes sync-specific types |
redis/event.py | Fixes async lock usage and adds async event types |
redis/background.py | Adds async recurring task scheduling capability |
redis/asyncio/multidb/ | Complete async implementation of multidb functionality |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
actual_message = await database.client.execute_command("ECHO" ,"healthcheck") | ||
return actual_message in expected_message | ||
else: | ||
# For a cluster checks if all nodes are healthy. | ||
all_nodes = database.client.get_nodes() | ||
for node in all_nodes: | ||
actual_message = await node.redis_connection.execute_command("ECHO" ,"healthcheck") |
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.
[nitpick] Missing space after comma in the ECHO command. Should be \"ECHO\", \"healthcheck\"
for consistency with Python style guidelines.
actual_message = await database.client.execute_command("ECHO" ,"healthcheck") | |
return actual_message in expected_message | |
else: | |
# For a cluster checks if all nodes are healthy. | |
all_nodes = database.client.get_nodes() | |
for node in all_nodes: | |
actual_message = await node.redis_connection.execute_command("ECHO" ,"healthcheck") | |
actual_message = await database.client.execute_command("ECHO", "healthcheck") | |
return actual_message in expected_message | |
else: | |
# For a cluster checks if all nodes are healthy. | |
all_nodes = database.client.get_nodes() | |
for node in all_nodes: | |
actual_message = await node.redis_connection.execute_command("ECHO", "healthcheck") |
Copilot uses AI. Check for mistakes.
# For a cluster checks if all nodes are healthy. | ||
all_nodes = database.client.get_nodes() | ||
for node in all_nodes: | ||
actual_message = await node.redis_connection.execute_command("ECHO" ,"healthcheck") |
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.
[nitpick] Missing space after comma in the ECHO command. Should be \"ECHO\", \"healthcheck\"
for consistency with Python style guidelines.
actual_message = await node.redis_connection.execute_command("ECHO" ,"healthcheck") | |
actual_message = await node.redis_connection.execute_command("ECHO", "healthcheck") |
Copilot uses AI. Check for mistakes.
r_multi_db, listener, config = r_multi_db | ||
|
||
event = asyncio.Event() | ||
asyncio.create_task(trigger_network_failure_action(fault_injector_client,config,event)) |
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.
[nitpick] Missing spaces after commas in function arguments. Should be trigger_network_failure_action(fault_injector_client, config, event)
for better readability.
asyncio.create_task(trigger_network_failure_action(fault_injector_client,config,event)) | |
asyncio.create_task(trigger_network_failure_action(fault_injector_client, config, event)) |
Copilot uses AI. Check for mistakes.
Pull Request check-list
Please make sure to review and check all of these items:
NOTE: these things are not required to open a PR and can be done
afterwards / while the PR is open.
Description of change
Please provide a description of the change here.