-
Notifications
You must be signed in to change notification settings - Fork 7
Add embedded FalkorDB lite mode support #164
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: main
Are you sure you want to change the base?
Changes from 13 commits
cb08776
6f581fc
b187279
67136fb
bab69e6
cb34d7c
a78af3a
a750095
0c328cd
87b0c11
f562609
e8f0e66
7a9e66e
1b1b8ab
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,3 +1,4 @@ | ||
| import asyncio | ||
| from typing import List, Optional, Union | ||
|
|
||
| import redis.asyncio as redis # type: ignore[import-not-found] | ||
|
|
@@ -31,6 +32,8 @@ class FalkorDB: | |
| print(node.properties['name']) | ||
| """ | ||
|
|
||
| _embedded_server = None | ||
|
|
||
| def __init__( | ||
| self, | ||
| host="localhost", | ||
|
|
@@ -70,41 +73,89 @@ def __init__( | |
| reinitialize_steps=5, | ||
| read_from_replicas=False, | ||
| address_remap=None, | ||
| embedded=False, | ||
| db_path=None, | ||
| embedded_config=None, | ||
| startup_timeout=10.0, | ||
| connection_acquire_timeout=5.0, | ||
| ): | ||
| self._embedded_server = None | ||
|
|
||
| if embedded: | ||
| from ..lite.server import EmbeddedServer | ||
|
|
||
| if max_connections is None: | ||
| max_connections = 16 | ||
|
|
||
| server = EmbeddedServer( | ||
| db_path=db_path, | ||
| config=embedded_config, | ||
| startup_timeout=startup_timeout, | ||
| ) | ||
| self._embedded_server = server | ||
| connection_pool = redis.BlockingConnectionPool( | ||
| connection_class=redis.UnixDomainSocketConnection, | ||
| path=server.unix_socket_path, | ||
| max_connections=max_connections, | ||
| timeout=connection_acquire_timeout, | ||
| socket_timeout=socket_timeout, | ||
| socket_connect_timeout=socket_connect_timeout, | ||
| socket_keepalive=socket_keepalive, | ||
| socket_keepalive_options=socket_keepalive_options, | ||
| encoding=encoding, | ||
| encoding_errors=encoding_errors, | ||
| decode_responses=True, | ||
| retry_on_error=retry_on_error, | ||
| retry=retry, | ||
| health_check_interval=health_check_interval, | ||
| client_name=client_name, | ||
| lib_name=lib_name, | ||
| lib_version=lib_version, | ||
| username=username, | ||
| password=password, | ||
| credential_provider=credential_provider, | ||
| protocol=protocol, | ||
| ) | ||
|
Comment on lines
+84
to
+118
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Blocking synchronous calls inside async
Consider wrapping the server startup in Example: async factory method`@classmethod`
async def create(cls, *, embedded=False, db_path=None, embedded_config=None,
startup_timeout=10.0, max_connections=None, **kwargs):
if embedded:
import asyncio
from ..lite.server import EmbeddedServer
if max_connections is None:
max_connections = 16
server = await asyncio.to_thread(
EmbeddedServer,
db_path=db_path,
config=embedded_config,
startup_timeout=startup_timeout,
)
kwargs["connection_pool"] = redis.BlockingConnectionPool(
connection_class=redis.UnixDomainSocketConnection,
path=server.unix_socket_path,
max_connections=max_connections,
...
)
instance = cls(**kwargs)
instance._embedded_server = server
return instance
else:
return cls(**kwargs)
coderabbitai[bot] marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
||
| 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, | ||
| decode_responses=True, | ||
| 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_data=ssl_ca_data, | ||
| ssl_check_hostname=ssl_check_hostname, | ||
| max_connections=max_connections, | ||
| single_connection_client=single_connection_client, | ||
| health_check_interval=health_check_interval, | ||
| client_name=client_name, | ||
| driver_info=DriverInfo(lib_name, lib_version or get_package_version()), | ||
| username=username, | ||
| retry=retry, | ||
| redis_connect_func=connect_func, | ||
| credential_provider=credential_provider, | ||
| protocol=protocol, | ||
| ) | ||
| if embedded: | ||
| conn = redis.Redis( | ||
| connection_pool=connection_pool, | ||
| single_connection_client=single_connection_client, | ||
| ) | ||
| else: | ||
| 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, | ||
| decode_responses=True, | ||
| 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_data=ssl_ca_data, | ||
| ssl_check_hostname=ssl_check_hostname, | ||
| max_connections=max_connections, | ||
| single_connection_client=single_connection_client, | ||
| health_check_interval=health_check_interval, | ||
| client_name=client_name, | ||
| driver_info=DriverInfo(lib_name, lib_version or get_package_version()), | ||
| username=username, | ||
| retry=retry, | ||
| redis_connect_func=connect_func, | ||
| credential_provider=credential_provider, | ||
| protocol=protocol, | ||
| ) | ||
|
|
||
| if Is_Cluster(conn): | ||
| conn = Cluster_Conn( | ||
|
|
@@ -215,16 +266,20 @@ async def config_set(self, name: str, value=None) -> None: | |
| return await self.connection.execute_command(CONFIG_CMD, "SET", name, value) | ||
|
|
||
| async def aclose(self) -> None: | ||
| """ | ||
| Close the underlying connection(s). | ||
| """ | ||
| """Close the underlying connection(s) and stop the embedded server | ||
| if present.""" | ||
|
|
||
| try: | ||
| await self.connection.aclose() | ||
| except RedisError: | ||
| # best-effort close — don't raise on Redis errors | ||
| pass | ||
|
|
||
| server = getattr(self, "_embedded_server", None) | ||
| if server is not None: | ||
| await asyncio.to_thread(server.stop) | ||
| self._embedded_server = None | ||
|
|
||
| async def __aenter__(self) -> "FalkorDB": | ||
| """Return self to support async with-statement usage.""" | ||
|
|
||
|
|
||
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.
AttributeError: 'FalkorDB' object has no attribute '_embedded_server'— CI test failure.Although
__init__setsself._embedded_server = Noneat line 80, the CI test fails withAttributeErrorat line 282, indicating instances are reachingaclose()without_embedded_serverset (e.g., viaobject.__new__, a patched__init__, or mock-created instances in existing tests). Add a class-level declaration as a safe default:🐛 Proposed fix
class FalkorDB: """ Asynchronous FalkorDB Class for interacting with a FalkorDB server. ... """ + _embedded_server = None + def __init__( self, ...🧰 Tools
🪛 GitHub Actions: Lint
[error] Ruff format check would reformat this file.
🤖 Prompt for AI Agents