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 24 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
57 changes: 49 additions & 8 deletions falkordb/cluster.py
Original file line number Diff line number Diff line change
@@ -1,18 +1,59 @@
from redis.cluster import RedisCluster
import redis.exceptions as redis_exceptions
import socket

# detect if a connection is a sentinel
def Is_Cluster(conn):
info = conn.info(section="server")
return "redis_mode" in info and info["redis_mode"] == "cluster"


# create a cluster connection from a Redis connection
def Cluster_Conn(conn, ssl):
# current sentinel
def Cluster_Conn(
conn,
ssl,
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,
):
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)
host = connection_kwargs.pop("host")
port = connection_kwargs.pop("port")
username = connection_kwargs.pop("username")
password = connection_kwargs.pop("password")

retry = connection_kwargs.pop("retry", None)
retry_on_timeout = connection_kwargs.pop("retry_on_timeout", None)
retry_on_error = connection_kwargs.pop(
"retry_on_error",
[
ConnectionRefusedError,
ConnectionError,
TimeoutError,
socket.timeout,
redis_exceptions.ConnectionError,
],
)
return RedisCluster(
host=host,
port=port,
username=username,
password=password,
ssl=ssl,
retry=retry,
retry_on_timeout=retry_on_timeout,
retry_on_error=retry_on_error,
require_full_coverage=require_full_coverage,
reinitialize_steps=reinitialize_steps,
read_from_replicas=read_from_replicas,
dynamic_startup_nodes=dynamic_startup_nodes,
url=url,
address_remap=address_remap,
startup_nodes=startup_nodes,
cluster_error_retry_attempts=cluster_error_retry_attempts,
)
214 changes: 125 additions & 89 deletions falkordb/falkordb.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,11 @@
from typing import List, Union

# config command
LIST_CMD = "GRAPH.LIST"
LIST_CMD = "GRAPH.LIST"
CONFIG_CMD = "GRAPH.CONFIG"

class FalkorDB():

class FalkorDB:
"""
FalkorDB Class for interacting with a FalkorDB server.

Expand All @@ -25,88 +26,121 @@ class FalkorDB():
"""

def __init__(
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
):

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)
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,
):
Comment on lines +29 to +78
Copy link

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.


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,
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

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.

credential_provider=credential_provider,
protocol=protocol,
)
Comment on lines +80 to +122
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

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.

Suggested change
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,
)


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, ssl=ssl,retry=retry, socket_connect_timeout=socket_connect_timeout)

if Is_Cluster(conn):
conn = Cluster_Conn(conn, ssl)

self.connection = conn
self.flushdb = conn.flushdb
conn = Cluster_Conn(
conn,
ssl,
cluster_error_retry_attempts,
startup_nodes,
require_full_coverage,
reinitialize_steps,
read_from_replicas,
dynamic_startup_nodes,
url,
address_remap,
)

self.connection = conn
self.flushdb = conn.flushdb
self.execute_command = conn.execute_command

@classmethod
Expand All @@ -129,18 +163,18 @@ def from_url(cls, url: str, **kwargs) -> "FalkorDB":
"""

# switch from redis:// to falkordb://
if url.startswith('falkor://'):
url = 'redis://' + url[len('falkor://'):]
elif url.startswith('falkors://'):
url = 'rediss://' + url[len('falkors://'):]
if url.startswith("falkor://"):
url = "redis://" + url[len("falkor://") :]
elif url.startswith("falkors://"):
url = "rediss://" + url[len("falkors://") :]

conn = redis.from_url(url, **kwargs)

connection_kwargs = conn.connection_pool.connection_kwargs
kwargs['host'] = connection_kwargs.get('host', 'localhost')
kwargs['port'] = connection_kwargs.get('port', 6379)
kwargs['username'] = connection_kwargs.get('username')
kwargs['password'] = connection_kwargs.get('password')
kwargs["host"] = connection_kwargs.get("host", "localhost")
kwargs["port"] = connection_kwargs.get("port", 6379)
kwargs["username"] = connection_kwargs.get("username")
kwargs["password"] = connection_kwargs.get("password")

# Initialize a FalkorDB instance using the updated kwargs
db = cls(**kwargs)
Expand All @@ -158,7 +192,9 @@ def select_graph(self, graph_id: str) -> Graph:
Graph: A new Graph instance associated with the selected graph.
"""
if not isinstance(graph_id, str) or graph_id == "":
raise TypeError(f"Expected a string parameter, but received {type(graph_id)}.")
raise TypeError(
f"Expected a string parameter, but received {type(graph_id)}."
)

return Graph(self, graph_id)

Expand All @@ -167,7 +203,7 @@ def list_graphs(self) -> List[str]:
Lists all graph names.
See: https://docs.falkordb.com/commands/graph.list.html

Returns:
Returns:
List: List of graph names.

"""
Expand Down
Loading