Skip to content
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

Open
wants to merge 29 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 12 commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
534723e
changed Cluster_Conn to use hostnames instead of ips
MuhammadQadora Sep 5, 2024
eff9ae8
changed redis version to use master until new release
MuhammadQadora Sep 5, 2024
3fd5c14
changed redis dependency to point to Dudi's branch
MuhammadQadora Sep 8, 2024
e5cfe80
for testing
MuhammadQadora Sep 8, 2024
377fae2
lowered retries
MuhammadQadora Sep 8, 2024
eef67b8
updated retry times
MuhammadQadora Sep 8, 2024
e65ee26
lowered retries
MuhammadQadora Sep 9, 2024
97bbe4a
insane retry count
MuhammadQadora Sep 10, 2024
0e79477
changed dependenct to debug
MuhammadQadora Sep 11, 2024
98719f6
reduced retry count
MuhammadQadora Sep 11, 2024
f65ce34
updated dependency
MuhammadQadora Sep 11, 2024
f623557
updated retry attempts
MuhammadQadora Sep 16, 2024
427ef70
Update falkordb.py
MuhammadQadora Sep 17, 2024
523ff53
Update cluster.py
MuhammadQadora Sep 17, 2024
8509151
Update pyproject.toml
MuhammadQadora Sep 17, 2024
09c969a
Update cluster.py
MuhammadQadora Sep 17, 2024
da45121
Update falkordb.py
MuhammadQadora Sep 19, 2024
14580bc
changed dependenc
MuhammadQadora Sep 19, 2024
ac39f29
Merge branch '57-changing-the-cluster_conn-function-to-use-domain-nam…
MuhammadQadora Sep 19, 2024
c074f70
Update falkordb.py
MuhammadQadora Sep 19, 2024
adbbada
Update cluster.py
MuhammadQadora Sep 19, 2024
670e908
changed dependenc
MuhammadQadora Sep 25, 2024
af46b2b
Merge branch '57-changing-the-cluster_conn-function-to-use-domain-nam…
MuhammadQadora Sep 25, 2024
d795189
Update falkordb.py
MuhammadQadora Sep 25, 2024
d05f644
Update falkordb.py
MuhammadQadora Sep 26, 2024
343f0c0
Update falkordb.py
MuhammadQadora Sep 29, 2024
a43db2e
Update falkordb.py
MuhammadQadora Oct 13, 2024
674c651
Update falkordb.py
MuhammadQadora Oct 13, 2024
4c485b4
Update falkordb.py
MuhammadQadora Oct 13, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 6 additions & 6 deletions falkordb/cluster.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
from redis.cluster import RedisCluster
from redis.cluster import RedisCluster, ClusterNode
from redis.retry import Retry
from redis.backoff import default_backoff

# detect if a connection is a sentinel
def Is_Cluster(conn):
Expand All @@ -7,12 +9,10 @@

# create a cluster connection from a Redis connection
def Cluster_Conn(conn, ssl):
# current sentinel
info = conn.execute_command("CLUSTER NODES")
nodes = [ ClusterNode(v['hostname'],k.split(':')[1]) for k,v in info.items()]

Check warning on line 13 in falkordb/cluster.py

View check run for this annotation

Codecov / codecov/patch

falkordb/cluster.py#L12-L13

Added lines #L12 - L13 were not covered by tests
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)

Check warning on line 17 in falkordb/cluster.py

View check run for this annotation

Codecov / codecov/patch

falkordb/cluster.py#L17

Added line #L17 was not covered by tests
Copy link

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


257 changes: 257 additions & 0 deletions poetry.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ packages = [{include = "falkordb"}]

[tool.poetry.dependencies]
python = "^3.8"
redis = "^5.0.1"
redis = {git = "https://github.com/MuhammadQadora/redis-py", rev = "master"}

[tool.poetry.group.test.dependencies]
pytest-cov = ">=4.1,<6.0"
Expand Down