-
Notifications
You must be signed in to change notification settings - Fork 3
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
57 changing the cluster conn function to use domain nameshostnames instead of ips #58
base: main
Are you sure you want to change the base?
Conversation
WalkthroughThe changes introduce significant enhancements to the Changes
Assessment against linked issues
Possibly related PRs
Suggested reviewers
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #58 +/- ##
==========================================
- Coverage 92.37% 92.27% -0.11%
==========================================
Files 35 35
Lines 2596 2601 +5
==========================================
+ Hits 2398 2400 +2
- Misses 198 201 +3 ☔ View full report in Codecov by Sentry. |
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.
Actionable comments posted: 2
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (2)
- falkordb/cluster.py (2 hunks)
- pyproject.toml (1 hunks)
Additional context used
GitHub Check: codecov/patch
falkordb/cluster.py
[warning] 12-13: falkordb/cluster.py#L12-L13
Added lines #L12 - L13 were not covered by tests
[warning] 17-17: falkordb/cluster.py#L17
Added line #L17 was not covered by tests
Additional comments not posted (1)
falkordb/cluster.py (1)
12-13
: Enhance theCluster_Conn
function with dynamic node retrieval.The use of the
CLUSTER NODES
command to dynamically retrieve node information is a significant improvement, allowing for more flexible and robust connection setups. However, ensure that theClusterNode
class is properly defined and integrated within the system.Tools
GitHub Check: codecov/patch
[warning] 12-13: falkordb/cluster.py#L12-L13
Added lines #L12 - L13 were not covered by tests
pyproject.toml
Outdated
@@ -13,7 +13,7 @@ packages = [{include = "falkordb"}] | |||
|
|||
[tool.poetry.dependencies] | |||
python = "^3.8" | |||
redis = "^5.0.1" | |||
redis = {git = "https://github.com/redis/redis-py", rev = "master"} |
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.
Consider the implications of using the master branch for the Redis dependency.
Switching the Redis dependency to the master branch of its Git repository can introduce instability due to the inclusion of potentially untested changes. It's generally safer to depend on a specific release or a stable branch to ensure compatibility and stability of the application.
Consider using a tagged release or a stable branch instead of the master branch:
-redis = {git = "https://github.com/redis/redis-py", rev = "master"}
+redis = {git = "https://github.com/redis/redis-py", rev = "stable"}
Committable suggestion was skipped due to low confidence.
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 need to wait for an official release from redis.
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.
@MuhammadQadora, understood! Once the official release from Redis is available, we can revisit the dependency to ensure stability. Let me know if you need any further assistance in the meantime!
falkordb/cluster.py
Outdated
username = connection_kwargs['username'] | ||
password = connection_kwargs['password'] | ||
|
||
return RedisCluster(host=host, port=port, username=username, password=password, ssl=ssl) | ||
return RedisCluster(Retry(default_backoff(),6),cluster_error_retry_attempts=6,startup_nodes=nodes,username=username,password=password,ssl=ssl) |
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.
Implement retry mechanism in the Cluster_Conn
function.
The addition of a retry mechanism using Retry
with a default backoff strategy enhances the resilience of the connection process. This is a good practice for distributed systems where transient errors are common. However, ensure that the new lines of code are covered by unit tests to maintain code quality and reliability.
The static analysis tool indicates that these lines are not covered by tests. Consider adding unit tests to cover these new functionalities.
# Example unit test for the retry mechanism
def test_retry_mechanism():
# Setup test
# Assert retry behavior
Tools
GitHub Check: codecov/patch
[warning] 17-17: falkordb/cluster.py#L17
Added line #L17 was not covered by tests
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.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (1)
poetry.lock
is excluded by!**/*.lock
Files selected for processing (1)
- pyproject.toml (1 hunks)
Files skipped from review as they are similar to previous changes (1)
- pyproject.toml
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.
Actionable comments posted: 1
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- falkordb/cluster.py (2 hunks)
Additional context used
GitHub Check: codecov/patch
falkordb/cluster.py
[warning] 12-13: falkordb/cluster.py#L12-L13
Added lines #L12 - L13 were not covered by tests
[warning] 17-17: falkordb/cluster.py#L17
Added line #L17 was not covered by tests
Additional comments not posted (2)
falkordb/cluster.py (2)
1-3
: Approved new imports.The imports of
RedisCluster
,ClusterNode
,Retry
, anddefault_backoff
are essential for the updated functionality of theCluster_Conn
function.
12-13
: Approved dynamic node retrieval.The use of the
CLUSTER NODES
command to dynamically retrieve node information is a significant improvement. Ensure that theClusterNode
instantiation is correctly handled.Run the following script to verify the correct instantiation of
ClusterNode
:Verification successful
Correct instantiation of
ClusterNode
verified.The
ClusterNode
is instantiated correctly using the hostname and port extracted from theCLUSTER NODES
command output. This confirms the intended functionality of dynamically retrieving node information.
ClusterNode
instantiation:ClusterNode(v['hostname'], k.split(':')[1])
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the correct instantiation of `ClusterNode`. # Test: Search for the instantiation pattern. Expect: Correct usage of hostname and port. rg --type python -A 5 $'ClusterNode'Length of output: 66
Script:
#!/bin/bash # Description: Verify the correct instantiation of `ClusterNode`. # Test: Search for the instantiation pattern. Expect: Correct usage of hostname and port. rg --type py -A 5 $'ClusterNode'Length of output: 844
Tools
GitHub Check: codecov/patch
[warning] 12-13: falkordb/cluster.py#L12-L13
Added lines #L12 - L13 were not covered by tests
falkordb/cluster.py
Outdated
username = connection_kwargs['username'] | ||
password = connection_kwargs['password'] | ||
|
||
return RedisCluster(host=host, port=port, username=username, password=password, ssl=ssl) | ||
return RedisCluster(Retry(default_backoff(),20),cluster_error_retry_attempts=6,startup_nodes=nodes,username=username,password=password,ssl=ssl) |
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.
Implement unit tests for new retry mechanism.
The addition of a retry mechanism using Retry
with a default backoff strategy enhances the resilience of the connection process. However, ensure that the new lines of code are covered by unit tests to maintain code quality and reliability.
Would you like me to help by generating the unit testing code or opening a GitHub issue to track this task?
Tools
GitHub Check: codecov/patch
[warning] 17-17: falkordb/cluster.py#L17
Added line #L17 was not covered by tests
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.
Actionable comments posted: 1
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- falkordb/cluster.py (2 hunks)
Additional context used
GitHub Check: codecov/patch
falkordb/cluster.py
[warning] 12-13: falkordb/cluster.py#L12-L13
Added lines #L12 - L13 were not covered by tests
[warning] 17-17: falkordb/cluster.py#L17
Added line #L17 was not covered by tests
Additional comments not posted (1)
falkordb/cluster.py (1)
1-3
: Imports are appropriate for the new functionality.The import statements are correctly aligned with the new functionality introduced in the
Cluster_Conn
function.
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.
Actionable comments posted: 1
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- falkordb/cluster.py (2 hunks)
Additional context used
GitHub Check: codecov/patch
falkordb/cluster.py
[warning] 12-13: falkordb/cluster.py#L12-L13
Added lines #L12 - L13 were not covered by tests
[warning] 17-17: falkordb/cluster.py#L17
Added line #L17 was not covered by tests
Additional comments not posted (1)
falkordb/cluster.py (1)
1-3
: Review of Import StatementsThe import statements for
RedisCluster
,ClusterNode
,Retry
, anddefault_backoff
are correctly added to support the new functionality in theCluster_Conn
function. These imports are essential for the implementation of the retry mechanism and the dynamic retrieval of cluster nodes.
falkordb/cluster.py
Outdated
info = conn.execute_command("CLUSTER NODES") | ||
nodes = [ ClusterNode(v['hostname'],k.split(':')[1]) for k,v in info.items()] | ||
connection_kwargs = conn.connection_pool.connection_kwargs | ||
host = connection_kwargs['host'] | ||
port = connection_kwargs['port'] | ||
username = connection_kwargs['username'] | ||
password = connection_kwargs['password'] | ||
|
||
return RedisCluster(host=host, port=port, username=username, password=password, ssl=ssl) | ||
return RedisCluster(Retry(default_backoff(),6),cluster_error_retry_attempts=20,startup_nodes=nodes,username=username,password=password,ssl=ssl) |
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.
Enhancements and Concerns in Cluster_Conn
Function
The modifications to Cluster_Conn
function are well-aligned with the PR objectives, transitioning from IP-based to hostname-based connections. The use of CLUSTER NODES
to dynamically retrieve node information and the implementation of a retry mechanism are significant improvements.
However, as indicated by static analysis tools and previous comments, these changes are not covered by unit tests. It is crucial to ensure that new functionalities work as expected and do not introduce regressions.
Would you like me to help by generating the unit testing code or opening a GitHub issue to track this task?
Tools
GitHub Check: codecov/patch
[warning] 12-13: falkordb/cluster.py#L12-L13
Added lines #L12 - L13 were not covered by tests
[warning] 17-17: falkordb/cluster.py#L17
Added line #L17 was not covered by tests
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.
Actionable comments posted: 1
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- falkordb/cluster.py (2 hunks)
Additional comments not posted (1)
falkordb/cluster.py (1)
1-3
: Imports are appropriate for the new functionality.The imports of
RedisCluster
,ClusterNode
,Retry
, anddefault_backoff
are necessary for the updatedCluster_Conn
function to function correctly.
falkordb/cluster.py
Outdated
info = conn.execute_command("CLUSTER NODES") | ||
nodes = [ ClusterNode(v['hostname'],k.split(':')[1]) for k,v in info.items()] | ||
connection_kwargs = conn.connection_pool.connection_kwargs | ||
host = connection_kwargs['host'] | ||
port = connection_kwargs['port'] | ||
username = connection_kwargs['username'] | ||
password = connection_kwargs['password'] | ||
|
||
return RedisCluster(host=host, port=port, username=username, password=password, ssl=ssl) | ||
return RedisCluster(Retry(default_backoff(),6),cluster_error_retry_attempts=6,startup_nodes=nodes,username=username,password=password,ssl=ssl) |
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.
Significant improvements in Cluster_Conn
but ensure unit tests are added.
The modifications to use the CLUSTER NODES
command and dynamically construct ClusterNode
instances are well-aligned with the PR objectives. The addition of a retry mechanism with a default backoff strategy enhances the resilience of the connection process.
However, as indicated by static analysis tools and previous comments, these changes are not covered by unit tests. It is crucial to ensure that new functionalities work as expected and do not introduce regressions.
Would you like me to help by generating the unit testing code or opening a GitHub issue to track this task?
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.
Actionable comments posted: 0
Outside diff range, codebase verification and nitpick comments (1)
falkordb/cluster.py (1)
Line range hint
1-19
: Significant improvements inCluster_Conn
but ensure unit tests are added.The modifications to use the
CLUSTER NODES
command and dynamically constructClusterNode
instances are well-aligned with the PR objectives. The addition of a retry mechanism with a default backoff strategy enhances the resilience of the connection process.However, as indicated by static analysis tools and previous comments, these changes are not covered by unit tests. It is crucial to ensure that new functionalities work as expected and do not introduce regressions.
Would you like me to help by generating the unit testing code or opening a GitHub issue to track this task?
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- falkordb/cluster.py (2 hunks)
Additional comments not posted (2)
falkordb/cluster.py (2)
12-13
: LGTM! The changes align with the PR objectives.The dynamic retrieval of Redis cluster node information using the
CLUSTER NODES
command and the construction ofClusterNode
instances are significant improvements. These changes enable the use of hostnames instead of IP addresses, aligning with the PR objectives.
17-17
: LGTM! The retry mechanism is a valuable addition.The utilization of the
Retry
mechanism with a default backoff strategy enhances the resilience of the connection process. This is a good practice for distributed systems where transient errors are common.The modification of the
RedisCluster
instantiation parameters to include thenodes
list aligns with the changes made to retrieve cluster node information dynamically.
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.
Actionable comments posted: 0
Outside diff range and nitpick comments (1)
falkordb/cluster.py (1)
12-13
: Significant improvements in retrieving cluster node information, but ensure unit tests are added.The modifications to use the
CLUSTER NODES
command and dynamically constructClusterNode
instances are well-aligned with the PR objectives. This change improves the flexibility and robustness of the connection setup process.However, as indicated by the static analysis tool and previous comments, these lines are not covered by unit tests. It is crucial to ensure that the new functionality works as expected and does not introduce regressions.
Would you like me to help by generating the unit testing code or opening a GitHub issue to track this task?
Tools
GitHub Check: codecov/patch
[warning] 12-13: falkordb/cluster.py#L12-L13
Added lines #L12 - L13 were not covered by tests
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (1)
poetry.lock
is excluded by!**/*.lock
Files selected for processing (1)
- falkordb/cluster.py (2 hunks)
Additional context used
GitHub Check: codecov/patch
falkordb/cluster.py
[warning] 12-13: falkordb/cluster.py#L12-L13
Added lines #L12 - L13 were not covered by tests
[warning] 17-17: falkordb/cluster.py#L17
Added line #L17 was not covered by tests
Additional comments not posted (1)
falkordb/cluster.py (1)
17-17
: Retry mechanism enhances fault-tolerance, but ensure unit tests are added.The addition of the
Retry
mechanism with a default backoff strategy improves the resilience of the connection process by allowing for retries in case of transient errors. This change aligns with best practices for distributed systems.However, as indicated by the static analysis tool and previous comments, this line is not covered by unit tests. It is crucial to ensure that the new functionality works as expected and does not introduce regressions.
Tools
GitHub Check: codecov/patch
[warning] 17-17: falkordb/cluster.py#L17
Added line #L17 was not covered by tests
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.
Actionable comments posted: 0
Outside diff range and nitpick comments (1)
falkordb/cluster.py (1)
Line range hint
1-49
: Significant improvements inCluster_Conn
but ensure unit tests are added.The modifications made to the
Cluster_Conn
function significantly enhance its capability to establish a Redis cluster connection. The handling of additional connection parameters and the implementation of a retry mechanism improve the flexibility and robustness of the connection setup.These changes reflect a substantial improvement in the function's design, enabling it to handle a wider range of connection scenarios and enhancing its adaptability to different Redis cluster configurations.
However, as mentioned in the previous comments, it is crucial to ensure that the new functionalities introduced in the
Cluster_Conn
function are thoroughly covered by unit tests. This will help maintain code quality, prevent regressions, and ensure the proper functioning of the enhanced connection capabilities.Would you like me to help by generating the unit testing code or opening a GitHub issue to track this task?
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (3)
- falkordb/cluster.py (1 hunks)
- falkordb/falkordb.py (1 hunks)
- pyproject.toml (1 hunks)
Files skipped from review as they are similar to previous changes (1)
- pyproject.toml
Additional comments not posted (4)
falkordb/cluster.py (3)
11-14
: LGTM!The extraction of connection parameters using the
pop
method is implemented correctly.
16-30
: Ensure unit tests are added for the new retry mechanism.The extraction of optional parameters, including the retry-related options, is implemented correctly. However, as mentioned in the previous comments, it is crucial to add unit tests to cover the new retry mechanism and ensure its proper functioning.
32-49
: Modifications toCluster_Conn
are well-aligned with PR objectives, but unit tests are still needed.The inclusion of the new parameters in the
RedisCluster
instantiation is consistent with the changes made to extract them from theconnection_kwargs
dictionary. The modifications to theCluster_Conn
function are well-aligned with the PR objectives of enhancing the Redis cluster connection capabilities.However, as mentioned in the previous comments, it is essential to ensure that the modified
Cluster_Conn
function is thoroughly covered by unit tests to maintain code quality and prevent regressions.falkordb/falkordb.py (1)
103-103
: Improved connection retry handlingThe addition of the
retry
parameter to themaster_for
method call is a positive change. It allows for configurable retry behavior when establishing a connection to the master node in a Redis Sentinel setup. This enhancement improves the resilience of the connection process, particularly in scenarios where transient network issues might occur.By passing the
retry
parameter from the constructor of theFalkorDB
class, users gain more control over the retry settings when initializing the database connection. This flexibility is beneficial for adapting to different network environments and ensuring a more robust connection mechanism.
…eshostnames-instead-of-ips' of github.com:FalkorDB/falkordb-py into 57-changing-the-cluster_conn-function-to-use-domain-nameshostnames-instead-of-ips
…eshostnames-instead-of-ips' of github.com:FalkorDB/falkordb-py into 57-changing-the-cluster_conn-function-to-use-domain-nameshostnames-instead-of-ips
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.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (7)
falkordb/cluster.py (3)
12-23
: Excellent enhancement of connection flexibility.The expanded function signature aligns well with the PR objectives, providing more granular control over cluster connections. The addition of parameters like
url
,address_remap
, anddynamic_startup_nodes
suggests improved support for complex network configurations.Consider updating the function's docstring to explain the purpose and expected values of each new parameter. This will greatly aid users of this function.
25-41
: Robust handling of connection and retry parameters.The extraction of connection parameters and the detailed handling of retry options, especially the comprehensive
retry_on_error
list, significantly enhance the function's error resilience and flexibility.Consider moving the list of exceptions for
retry_on_error
to a module-level constant. This would improve readability and make it easier to maintain or extend in the future.RETRY_ON_ERROR_EXCEPTIONS = [ ConnectionRefusedError, ConnectionError, TimeoutError, socket.timeout, redis_exceptions.ConnectionError, ] # Then in the function: retry_on_error = connection_kwargs.pop("retry_on_error", RETRY_ON_ERROR_EXCEPTIONS)
1-59
: Excellent enhancements to Cluster_Conn, consider adding unit tests.The changes to the
Cluster_Conn
function significantly improve its flexibility and error handling capabilities. The addition of new parameters for detailed cluster configuration and the implementation of retry mechanisms align well with the PR objectives.To ensure the reliability of these new features, it would be beneficial to add unit tests covering the new functionality, especially:
- The handling of new parameters
- The retry mechanism
- Error scenarios with the new exception handling
Would you like me to help by generating some unit test templates or opening a GitHub issue to track this task?
falkordb/falkordb.py (4)
69-77
: LGTM! Consider adding parameter documentation.The new parameters added to the
__init__
method align well with the PR objective of enhancing cluster connection functionality. They provide greater flexibility in configuring cluster connections while maintaining backward compatibility with default values.Consider adding docstring comments for these new parameters to improve code documentation and make it easier for users to understand their purpose and usage.
126-126
: LGTM! Consider additional error handling.The addition of the
retry
parameter to thesentinel.master_for
method call improves connection reliability, which is a positive change.Consider wrapping this call in a try-except block to handle potential connection errors gracefully. This would further improve the robustness of the connection process.
129-140
: LGTM! Consider improving import statements.The changes in the
Cluster_Conn
function call correctly pass all the new cluster configuration options, implementing the PR objective of enhancing cluster connection functionality.Consider replacing the star import
from .cluster import *
with explicit imports of the required functions (e.g.,from .cluster import Cluster_Conn, Is_Cluster
). This would improve code readability and maintainability by making it clear which functions are being used from thecluster
module.🧰 Tools
Ruff
129-129:
Cluster_Conn
may be undefined, or defined from star imports(F405)
Line range hint
2-3
: Address potential undefined variables warnings.The static analysis tool Ruff has reported potential undefined variables:
Is_Sentinel
,Sentinel_Conn
,Is_Cluster
, andCluster_Conn
. These warnings are likely due to the star imports from.cluster
and.sentinel
modules.To resolve these warnings and improve code clarity:
- Replace the star imports with explicit imports:
from .cluster import Is_Cluster, Cluster_Conn from .sentinel import Is_Sentinel, Sentinel_Conn
- If these functions are meant to be part of the public API of the
falkordb
package, consider adding them to the__all__
list in the respective modules.This change will make the code more explicit about its dependencies and reduce the risk of naming conflicts.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
🔇 Files ignored due to path filters (1)
poetry.lock
is excluded by!**/*.lock
📒 Files selected for processing (3)
- falkordb/cluster.py (1 hunks)
- falkordb/falkordb.py (5 hunks)
- pyproject.toml (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- pyproject.toml
🧰 Additional context used
Ruff
falkordb/falkordb.py
124-124:
Is_Sentinel
may be undefined, or defined from star imports(F405)
125-125:
Sentinel_Conn
may be undefined, or defined from star imports(F405)
128-128:
Is_Cluster
may be undefined, or defined from star imports(F405)
129-129:
Cluster_Conn
may be undefined, or defined from star imports(F405)
🔇 Additional comments not posted (4)
falkordb/cluster.py (2)
2-2
: LGTM: Appropriate import for exception handling.The addition of the import statement for redis exceptions is consistent with the enhanced error handling in the
Cluster_Conn
function. This is a good practice as it allows for more specific exception handling.
42-59
: Comprehensive RedisCluster instantiation with new parameters.The RedisCluster instantiation correctly incorporates all the new parameters, enhancing the flexibility and configurability of the cluster connection. The order of parameters is logical and easy to follow.
To ensure that all new parameters are being used correctly, please run the following verification script:
This script will help confirm that all new parameters introduced in the
Cluster_Conn
function signature are indeed being passed to theRedisCluster
constructor.✅ Verification successful
RedisCluster instantiation correctly uses all new parameters.
All the new parameters introduced in the
Cluster_Conn
function are successfully passed to theRedisCluster
constructor, ensuring enhanced flexibility and configurability of the cluster connection.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that all new parameters in Cluster_Conn are used in RedisCluster instantiation # Test: Check if all new parameters are present in the RedisCluster instantiation rg --type python -e 'def Cluster_Conn' -A 50 falkordb/cluster.py | grep -E '(cluster_error_retry_attempts|startup_nodes|require_full_coverage|reinitialize_steps|read_from_replicas|dynamic_startup_nodes|url|address_remap)='Length of output: 255
Script:
#!/bin/bash # Description: Verify that all new parameters in Cluster_Conn are used in RedisCluster instantiation # Use fd to find the cluster.py file and then grep for the parameter assignments fd 'cluster\.py$' . --type f --exec rg -A 50 'def Cluster_Conn' {} | grep -E '(cluster_error_retry_attempts|startup_nodes|require_full_coverage|reinitialize_steps|read_from_replicas|dynamic_startup_nodes|url|address_remap)='Length of output: 805
falkordb/falkordb.py (2)
80-122
: LGTM! Consistent parameter passing.The changes in the
redis.Redis
instantiation correctly pass all the new configuration options to the underlying Redis connection. This ensures consistency between theFalkorDB
class interface and the Redis connection, which is crucial for proper functionality.
Line range hint
1-241
: Overall, great improvements to cluster connection functionality!The changes in this file significantly enhance the
FalkorDB
class's ability to handle cluster connections, aligning well with the PR objectives. The new parameters and their integration into the connection logic provide greater flexibility and configurability.Key points:
- New cluster-related parameters have been added to the
__init__
method.- These parameters are correctly passed to the underlying Redis and Cluster connections.
- The changes maintain backward compatibility while introducing new features.
Suggestions for further improvement:
- Add parameter documentation for the new
__init__
method parameters.- Consider additional error handling for the Sentinel connection.
- Replace star imports with explicit imports to improve code clarity and address static analysis warnings.
Great work on enhancing the FalkorDB functionality!
🧰 Tools
Ruff
124-124:
Is_Sentinel
may be undefined, or defined from star imports(F405)
125-125:
Sentinel_Conn
may be undefined, or defined from star imports(F405)
128-128:
Is_Cluster
may be undefined, or defined from star imports(F405)
129-129:
Cluster_Conn
may be undefined, or defined from star imports(F405)
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.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (4)
falkordb/falkordb.py (4)
29-78
: LGTM! Consider adding type hints for new parameters.The additions to the
__init__
method signature align well with the PR objective of enhancing cluster connection functionality. The new parameters provide greater flexibility in configuring cluster connections, which is a positive improvement.Consider adding type hints for the new parameters to improve code readability and maintainability. For example:
def __init__( self, # ... existing parameters ... cluster_error_retry_attempts: int = 3, startup_nodes: Optional[List[Dict[str, Union[str, int]]]] = None, require_full_coverage: bool = False, # ... other new parameters ... ):
129-140
: LGTM! Cluster connection enhancement implemented correctly.The updates to the
Cluster_Conn
function call directly address the PR objective of enhancing cluster connection functionality. The new parameters allow for more flexible and robust cluster configurations, which is a significant improvement.Consider adding a comment explaining the purpose of these new parameters, especially for less obvious ones like
reinitialize_steps
ordynamic_startup_nodes
. This would improve code maintainability and help future developers understand the configuration options.🧰 Tools
🪛 Ruff
129-129:
Cluster_Conn
may be undefined, or defined from star imports(F405)
166-177
: LGTM! Improved URL handling and instance initialization.The updates to the
from_url
method are well-implemented. The support for FalkorDB-specific URL schemes ('falkor://' and 'falkors://') improves flexibility, and the use of connection kwargs to initialize a FalkorDB instance ensures consistency with the new parameters.Consider adding error handling for unsupported URL schemes. For example:
if url.startswith("falkor://"): url = "redis://" + url[len("falkor://"):] elif url.startswith("falkors://"): url = "rediss://" + url[len("falkors://"):] elif not url.startswith(("redis://", "rediss://", "unix://")): raise ValueError(f"Unsupported URL scheme: {url}")This would provide clearer feedback if an invalid URL scheme is used.
124-129
: Consider explicit imports to resolve potential undefined variable warnings.The static analysis tool has flagged potential issues with undefined variables (
Is_Sentinel
,Sentinel_Conn
,Is_Cluster
, andCluster_Conn
). These are likely false positives due to star imports. However, to improve code clarity and resolve these warnings, consider using explicit imports instead of star imports.Replace the star import:
from .cluster import * from .sentinel import *with explicit imports:
from .cluster import Is_Cluster, Cluster_Conn from .sentinel import Is_Sentinel, Sentinel_ConnThis change would make the code more explicit about its dependencies and resolve the static analysis warnings.
🧰 Tools
🪛 Ruff
124-124:
Is_Sentinel
may be undefined, or defined from star imports(F405)
125-125:
Sentinel_Conn
may be undefined, or defined from star imports(F405)
128-128:
Is_Cluster
may be undefined, or defined from star imports(F405)
129-129:
Cluster_Conn
may be undefined, or defined from star imports(F405)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
- falkordb/falkordb.py (5 hunks)
🧰 Additional context used
🪛 Ruff
falkordb/falkordb.py
124-124:
Is_Sentinel
may be undefined, or defined from star imports(F405)
125-125:
Sentinel_Conn
may be undefined, or defined from star imports(F405)
128-128:
Is_Cluster
may be undefined, or defined from star imports(F405)
129-129:
Cluster_Conn
may be undefined, or defined from star imports(F405)
🔇 Additional comments (3)
falkordb/falkordb.py (3)
80-122
: LGTM! Existing connection initialization preserved.The Redis connection initialization remains unchanged, which is appropriate as the new parameters are specific to cluster configuration. This approach maintains compatibility with non-cluster scenarios.
126-126
: LGTM! Enhanced Sentinel connection resilience.The addition of the
retry
parameter to themaster_for
method call is a good improvement. It enhances the resilience of Sentinel connections, aligning with the overall goal of improving connection handling.
Line range hint
1-241
: Overall, excellent implementation of cluster connection enhancements.The changes in this file successfully address the PR objective of enhancing cluster connection functionality. The additions to the
__init__
method, updates to cluster and sentinel connections, and improvements in URL handling all contribute to a more flexible and robustFalkorDB
class.Key improvements:
- Added new parameters for fine-grained cluster configuration.
- Enhanced Sentinel connection resilience with the
retry
parameter.- Updated
from_url
method to support FalkorDB-specific URL schemes.Minor suggestions have been made to further improve code quality, including adding type hints, improving documentation, and resolving potential import issues.
Great work on this implementation!
🧰 Tools
🪛 Ruff
124-124:
Is_Sentinel
may be undefined, or defined from star imports(F405)
125-125:
Sentinel_Conn
may be undefined, or defined from star imports(F405)
128-128:
Is_Cluster
may be undefined, or defined from star imports(F405)
129-129:
Cluster_Conn
may be undefined, or defined from star imports(F405)
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.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (4)
falkordb/falkordb.py (4)
69-77
: LGTM! Consider adding parameter descriptions.The new cluster-related parameters enhance the flexibility of the
FalkorDB
class, aligning well with the PR objectives. These additions will allow for more fine-grained control over cluster connections.Consider adding parameter descriptions in the docstring for these new parameters to improve code documentation and usability.
129-140
: LGTM! Consider using keyword arguments for clarity.The updated
Cluster_Conn
function call with new parameters aligns well with the PR objectives and allows for more flexible cluster configuration. This change enhances the connection method as intended.For improved readability, consider using keyword arguments when calling
Cluster_Conn
. This would make the function call more self-documenting, especially with the increased number of parameters. For example:conn = Cluster_Conn( conn, ssl=ssl, cluster_error_retry_attempts=cluster_error_retry_attempts, startup_nodes=startup_nodes, require_full_coverage=require_full_coverage, # ... (other parameters) )🧰 Tools
🪛 Ruff
129-129:
Cluster_Conn
may be undefined, or defined from star imports(F405)
166-169
: Good addition of FalkorDB-specific URL handling.The new logic for handling "falkor://" and "falkors://" URL schemes is a good addition. It allows users to use FalkorDB-specific URL schemes while maintaining compatibility with the underlying Redis connection.
Consider adding an
else
clause to handle unexpected URL schemes. This would improve error handling and provide clearer feedback to users. For example:if url.startswith("falkor://"): url = "redis://" + url[len("falkor://"):] elif url.startswith("falkors://"): url = "rediss://" + url[len("falkors://"):] else: raise ValueError(f"Unsupported URL scheme: {url}")
Line range hint
2-3
: Consider explicit imports for better code clarity.The static analysis tool Ruff reports potential undefined names:
Is_Sentinel
,Sentinel_Conn
,Is_Cluster
, andCluster_Conn
. These are likely imported from thecluster
andsentinel
modules using star imports.While star imports can simplify the API, they may lead to namespace pollution and make it harder to track the origin of functions. Consider using explicit imports for better code clarity and maintainability. For example:
from .cluster import Is_Cluster, Cluster_Conn from .sentinel import Is_Sentinel, Sentinel_ConnThis change would make it clearer where these functions are coming from and could help prevent potential naming conflicts in the future.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
- falkordb/falkordb.py (5 hunks)
🧰 Additional context used
🪛 Ruff
falkordb/falkordb.py
124-124:
Is_Sentinel
may be undefined, or defined from star imports(F405)
125-125:
Sentinel_Conn
may be undefined, or defined from star imports(F405)
128-128:
Is_Cluster
may be undefined, or defined from star imports(F405)
129-129:
Cluster_Conn
may be undefined, or defined from star imports(F405)
🔇 Additional comments (2)
falkordb/falkordb.py (2)
126-126
: Good addition of retry parameter.Adding the
retry
parameter to thesentinel.master_for
method call enhances error handling for sentinel connections. This change improves the robustness of the connection process and is consistent with theretry
parameter passed to the Redis connection.
Line range hint
1-241
: Overall, excellent implementation of cluster connection enhancements.The changes in this file successfully address the PR objectives by enhancing the
Cluster_Conn
function to support hostnames and providing more flexible configuration options. The implementation is well-structured and maintains compatibility with existing code.Key improvements:
- Added new cluster-related parameters to the
__init__
method.- Updated the
Cluster_Conn
function call with new parameters.- Enhanced URL handling to support FalkorDB-specific schemes.
These changes will significantly improve the flexibility and usability of FalkorDB cluster connections.
To further enhance the code:
- Consider adding parameter descriptions in the docstring for the new cluster-related parameters.
- Use keyword arguments in the
Cluster_Conn
function call for better readability.- Improve error handling in the URL scheme conversion.
- Consider using explicit imports instead of star imports for better code clarity.
Great work on implementing these enhancements!
🧰 Tools
🪛 Ruff
124-124:
Is_Sentinel
may be undefined, or defined from star imports(F405)
125-125:
Sentinel_Conn
may be undefined, or defined from star imports(F405)
128-128:
Is_Cluster
may be undefined, or defined from star imports(F405)
129-129:
Cluster_Conn
may be undefined, or defined from star imports(F405)
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.
Actionable comments posted: 2
🧹 Outside diff range and nitpick comments (1)
falkordb/falkordb.py (1)
Line range hint
2-3
: Avoid wildcard imports for better clarity and maintainability.Using
from .cluster import *
andfrom .sentinel import *
can pollute the namespace and make it difficult to track where specific functions or classes are coming from. It's best practice to explicitly import only the necessary components.Consider updating the imports as follows:
-from .cluster import * -from .sentinel import * +from .cluster import Cluster_Conn, Is_Cluster +from .sentinel import Sentinel_Conn, Is_Sentinel
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
- falkordb/falkordb.py (5 hunks)
🧰 Additional context used
🪛 Ruff
falkordb/falkordb.py
124-124:
Is_Sentinel
may be undefined, or defined from star imports(F405)
125-125:
Sentinel_Conn
may be undefined, or defined from star imports(F405)
128-128:
Is_Cluster
may be undefined, or defined from star imports(F405)
129-129:
Cluster_Conn
may be undefined, or defined from star imports(F405)
🔇 Additional comments (2)
falkordb/falkordb.py (2)
12-12
: Cleaned up class definition.The removal of unnecessary parentheses in the class definition improves readability. Defining the class as
class FalkorDB:
is appropriate when not inheriting from any superclass.
80-122
: Verify the validity of parameters passed toredis.Redis
.Ensure that all parameters passed to
redis.Redis
are valid and supported by the Redis client library. Passing unsupported parameters can lead to runtime errors.You can confirm the valid parameters by checking the Redis client's documentation or by running the following command:
self, | ||
host="localhost", | ||
port=6379, | ||
password=None, | ||
socket_timeout=None, | ||
socket_connect_timeout=None, | ||
socket_keepalive=None, | ||
socket_keepalive_options=None, | ||
connection_pool=None, | ||
unix_socket_path=None, | ||
encoding="utf-8", | ||
encoding_errors="strict", | ||
charset=None, | ||
errors=None, | ||
retry_on_timeout=False, | ||
retry_on_error=None, | ||
ssl=False, | ||
ssl_keyfile=None, | ||
ssl_certfile=None, | ||
ssl_cert_reqs="required", | ||
ssl_ca_certs=None, | ||
ssl_ca_path=None, | ||
ssl_ca_data=None, | ||
ssl_check_hostname=False, | ||
ssl_password=None, | ||
ssl_validate_ocsp=False, | ||
ssl_validate_ocsp_stapled=False, | ||
ssl_ocsp_context=None, | ||
ssl_ocsp_expected_cert=None, | ||
max_connections=None, | ||
single_connection_client=False, | ||
health_check_interval=0, | ||
client_name=None, | ||
lib_name="FalkorDB", | ||
lib_version="1.0.0", | ||
username=None, | ||
retry=None, | ||
connect_func=None, | ||
credential_provider=None, | ||
protocol=2, | ||
# FalkorDB Cluster Params | ||
cluster_error_retry_attempts=3, | ||
startup_nodes=None, | ||
require_full_coverage=False, | ||
reinitialize_steps=5, | ||
read_from_replicas=False, | ||
dynamic_startup_nodes=True, | ||
url=None, | ||
address_remap=None, | ||
): |
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.
🛠️ Refactor suggestion
Refactor __init__
parameters for enhanced maintainability.
The __init__
method has a lengthy list of parameters, which can make the code harder to read and maintain. Consider grouping related parameters into a configuration object or using **kwargs
to simplify the method signature and improve readability.
falkordb/falkordb.py
Outdated
if Is_Sentinel(conn): | ||
self.sentinel, self.service_name = Sentinel_Conn(conn, ssl) | ||
conn = self.sentinel.master_for(self.service_name, ssl=ssl) | ||
conn = self.sentinel.master_for(self.service_name, retry=retry, retry_on_error=retry_on_error) | ||
|
||
if Is_Cluster(conn): | ||
conn = Cluster_Conn(conn, ssl) | ||
|
||
self.connection = conn | ||
self.flushdb = conn.flushdb | ||
conn = Cluster_Conn( |
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.
Ensure proper import of Is_Sentinel
, Sentinel_Conn
, Is_Cluster
, and Cluster_Conn
.
These functions may be undefined due to the use of wildcard imports. This can cause NameError
exceptions at runtime.
After updating the imports as previously suggested, this issue should be resolved.
🧰 Tools
🪛 Ruff
124-124:
Is_Sentinel
may be undefined, or defined from star imports(F405)
125-125:
Sentinel_Conn
may be undefined, or defined from star imports(F405)
128-128:
Is_Cluster
may be undefined, or defined from star imports(F405)
129-129:
Cluster_Conn
may be undefined, or defined from star imports(F405)
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.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
- falkordb/falkordb.py (5 hunks)
🧰 Additional context used
🪛 Ruff
falkordb/falkordb.py
124-124:
Is_Sentinel
may be undefined, or defined from star imports(F405)
125-125:
Sentinel_Conn
may be undefined, or defined from star imports(F405)
128-128:
Is_Cluster
may be undefined, or defined from star imports(F405)
129-129:
Cluster_Conn
may be undefined, or defined from star imports(F405)
🔇 Additional comments (2)
falkordb/falkordb.py (2)
70-77
: Enhanced__init__
with cluster configuration parametersThe addition of cluster-related parameters to the
__init__
method enhances the flexibility and configurability of theFalkorDB
class, allowing for more precise control over cluster connections.
129-140
: Updated cluster connection using new parametersThe
Cluster_Conn
function now incorporates the new cluster parameters, ensuring that the cluster connection is established with the specified configurations.🧰 Tools
🪛 Ruff
129-129:
Cluster_Conn
may be undefined, or defined from star imports(F405)
lib_version=lib_version, | ||
username=username, | ||
retry=retry, | ||
redis_connect_func=connect_func, |
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.
Potential issue: Invalid parameter redis_connect_func
The redis.Redis
constructor does not have a parameter named redis_connect_func
. Including this parameter may lead to a TypeError
at runtime.
Apply this diff to fix the issue:
username=username,
retry=retry,
- redis_connect_func=connect_func,
Please verify if connect_func
is required, and remove or replace it with the correct parameter if necessary.
Committable suggestion was skipped due to low confidence.
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.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (3)
falkordb/falkordb.py (3)
12-12
: Approve class definition change and suggest parameter grouping for__init__
.The simplified class definition is a good style improvement. However, the
__init__
method now has a very long list of parameters, which could make the code harder to maintain. Consider grouping related parameters into configuration objects or using**kwargs
to simplify the method signature.Here's a suggestion to improve maintainability:
def __init__( self, host="localhost", port=6379, **kwargs ): # Group related parameters redis_config = { 'password': kwargs.get('password'), 'socket_timeout': kwargs.get('socket_timeout'), # ... other Redis-specific parameters } cluster_config = { 'cluster_error_retry_attempts': kwargs.get('cluster_error_retry_attempts', 3), 'startup_nodes': kwargs.get('startup_nodes'), # ... other cluster-specific parameters } # Use the grouped configurations in your connection logic # ...This approach would make the method signature more concise while still allowing for all the configuration options.
Also applies to: 29-78
124-126
: Approve Sentinel connection changes and suggest import improvements.The changes to Sentinel connection handling, including the addition of
ssl
,retry
, andretry_on_error
parameters, are appropriate and align with the enhanced configuration options.To improve code clarity and avoid potential naming conflicts, consider replacing the wildcard imports with explicit imports:
from .sentinel import Is_Sentinel, Sentinel_ConnThis change will make it clear where these functions are coming from and reduce the risk of naming conflicts.
🧰 Tools
🪛 Ruff
124-124:
Is_Sentinel
may be undefined, or defined from star imports(F405)
125-125:
Sentinel_Conn
may be undefined, or defined from star imports(F405)
128-140
: Approve Cluster connection changes and suggest import improvements.The changes to Cluster connection handling, including the addition of numerous new parameters, are appropriate and align with the PR objective of enhancing cluster connection functionality.
To improve code clarity and avoid potential naming conflicts, consider replacing the wildcard imports with explicit imports:
from .cluster import Is_Cluster, Cluster_ConnThis change will make it clear where these functions are coming from and reduce the risk of naming conflicts.
🧰 Tools
🪛 Ruff
128-128:
Is_Cluster
may be undefined, or defined from star imports(F405)
129-129:
Cluster_Conn
may be undefined, or defined from star imports(F405)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
- falkordb/falkordb.py (5 hunks)
🧰 Additional context used
🪛 Ruff
falkordb/falkordb.py
124-124:
Is_Sentinel
may be undefined, or defined from star imports(F405)
125-125:
Sentinel_Conn
may be undefined, or defined from star imports(F405)
128-128:
Is_Cluster
may be undefined, or defined from star imports(F405)
129-129:
Cluster_Conn
may be undefined, or defined from star imports(F405)
🔇 Additional comments (1)
falkordb/falkordb.py (1)
166-177
: Approve changes tofrom_url
method.The additions to handle
falkor://
andfalkors://
URL schemes, along with the extraction of connection parameters from the Redis connection, are well-implemented. These changes improve the usability of the library and ensure correct initialization of the FalkorDB instance.
conn = redis.Redis( | ||
host=host, | ||
port=port, | ||
db=0, | ||
password=password, | ||
socket_timeout=socket_timeout, | ||
socket_connect_timeout=socket_connect_timeout, | ||
socket_keepalive=socket_keepalive, | ||
socket_keepalive_options=socket_keepalive_options, | ||
connection_pool=connection_pool, | ||
unix_socket_path=unix_socket_path, | ||
encoding=encoding, | ||
encoding_errors=encoding_errors, | ||
charset=charset, | ||
errors=errors, | ||
decode_responses=True, | ||
retry_on_timeout=retry_on_timeout, | ||
retry_on_error=retry_on_error, | ||
ssl=ssl, | ||
ssl_keyfile=ssl_keyfile, | ||
ssl_certfile=ssl_certfile, | ||
ssl_cert_reqs=ssl_cert_reqs, | ||
ssl_ca_certs=ssl_ca_certs, | ||
ssl_ca_path=ssl_ca_path, | ||
ssl_ca_data=ssl_ca_data, | ||
ssl_check_hostname=ssl_check_hostname, | ||
ssl_password=ssl_password, | ||
ssl_validate_ocsp=ssl_validate_ocsp, | ||
ssl_validate_ocsp_stapled=ssl_validate_ocsp_stapled, | ||
ssl_ocsp_context=ssl_ocsp_context, | ||
ssl_ocsp_expected_cert=ssl_ocsp_expected_cert, | ||
max_connections=max_connections, | ||
single_connection_client=single_connection_client, | ||
health_check_interval=health_check_interval, | ||
client_name=client_name, | ||
lib_name=lib_name, | ||
lib_version=lib_version, | ||
username=username, | ||
retry=retry, | ||
redis_connect_func=connect_func, | ||
credential_provider=credential_provider, | ||
protocol=protocol, | ||
) |
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.
Remove invalid parameter redis_connect_func
from Redis initialization.
The redis.Redis
constructor does not have a parameter named redis_connect_func
. Including this parameter may lead to a TypeError
at runtime.
Apply this diff to fix the issue:
- redis_connect_func=connect_func,
If connect_func
is intended for custom connection logic, consider using it before initializing the Redis connection or explore if it can be used with the connection_pool
parameter.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
conn = redis.Redis( | |
host=host, | |
port=port, | |
db=0, | |
password=password, | |
socket_timeout=socket_timeout, | |
socket_connect_timeout=socket_connect_timeout, | |
socket_keepalive=socket_keepalive, | |
socket_keepalive_options=socket_keepalive_options, | |
connection_pool=connection_pool, | |
unix_socket_path=unix_socket_path, | |
encoding=encoding, | |
encoding_errors=encoding_errors, | |
charset=charset, | |
errors=errors, | |
decode_responses=True, | |
retry_on_timeout=retry_on_timeout, | |
retry_on_error=retry_on_error, | |
ssl=ssl, | |
ssl_keyfile=ssl_keyfile, | |
ssl_certfile=ssl_certfile, | |
ssl_cert_reqs=ssl_cert_reqs, | |
ssl_ca_certs=ssl_ca_certs, | |
ssl_ca_path=ssl_ca_path, | |
ssl_ca_data=ssl_ca_data, | |
ssl_check_hostname=ssl_check_hostname, | |
ssl_password=ssl_password, | |
ssl_validate_ocsp=ssl_validate_ocsp, | |
ssl_validate_ocsp_stapled=ssl_validate_ocsp_stapled, | |
ssl_ocsp_context=ssl_ocsp_context, | |
ssl_ocsp_expected_cert=ssl_ocsp_expected_cert, | |
max_connections=max_connections, | |
single_connection_client=single_connection_client, | |
health_check_interval=health_check_interval, | |
client_name=client_name, | |
lib_name=lib_name, | |
lib_version=lib_version, | |
username=username, | |
retry=retry, | |
redis_connect_func=connect_func, | |
credential_provider=credential_provider, | |
protocol=protocol, | |
) | |
conn = redis.Redis( | |
host=host, | |
port=port, | |
db=0, | |
password=password, | |
socket_timeout=socket_timeout, | |
socket_connect_timeout=socket_connect_timeout, | |
socket_keepalive=socket_keepalive, | |
socket_keepalive_options=socket_keepalive_options, | |
connection_pool=connection_pool, | |
unix_socket_path=unix_socket_path, | |
encoding=encoding, | |
encoding_errors=encoding_errors, | |
charset=charset, | |
errors=errors, | |
decode_responses=True, | |
retry_on_timeout=retry_on_timeout, | |
retry_on_error=retry_on_error, | |
ssl=ssl, | |
ssl_keyfile=ssl_keyfile, | |
ssl_certfile=ssl_certfile, | |
ssl_cert_reqs=ssl_cert_reqs, | |
ssl_ca_certs=ssl_ca_certs, | |
ssl_ca_path=ssl_ca_path, | |
ssl_ca_data=ssl_ca_data, | |
ssl_check_hostname=ssl_check_hostname, | |
ssl_password=ssl_password, | |
ssl_validate_ocsp=ssl_validate_ocsp, | |
ssl_validate_ocsp_stapled=ssl_validate_ocsp_stapled, | |
ssl_ocsp_context=ssl_ocsp_context, | |
ssl_ocsp_expected_cert=ssl_ocsp_expected_cert, | |
max_connections=max_connections, | |
single_connection_client=single_connection_client, | |
health_check_interval=health_check_interval, | |
client_name=client_name, | |
lib_name=lib_name, | |
lib_version=lib_version, | |
username=username, | |
retry=retry, | |
credential_provider=credential_provider, | |
protocol=protocol, | |
) |
fix #57
Changed the Cluster_Conn function to use hostnames instead of ips
Summary by CodeRabbit
New Features
Bug Fixes
Chores