-
Notifications
You must be signed in to change notification settings - Fork 41
Add in min_connections to available arguments #266
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 all commits
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 | ||||
|---|---|---|---|---|---|---|
|
|
@@ -984,6 +984,16 @@ class ConnectionPool: | |||||
|
|
||||||
| Any additional keyword arguments are passed to the constructor of | ||||||
| ``connection_class``. | ||||||
|
|
||||||
| Args: | ||||||
| connection_class: The class to use for creating connections. | ||||||
| Defaults to :py:class:`~valkey.asyncio.connection.Connection`. | ||||||
| max_connections: The maximum number of connections to create. | ||||||
| If not set, there is no limit. | ||||||
| min_connections: The minimum number of connections to pre-create | ||||||
| when the pool is initialized. Call :meth:`initialize` to eagerly | ||||||
| connect them, or they will be connected lazily on first use. | ||||||
| Must be less than or equal to ``max_connections``. Defaults to 0. | ||||||
| """ | ||||||
|
|
||||||
| @classmethod | ||||||
|
|
@@ -1036,20 +1046,43 @@ def __init__( | |||||
| self, | ||||||
| connection_class: Type[AbstractConnection] = Connection, | ||||||
| max_connections: Optional[int] = None, | ||||||
| min_connections: int = 0, | ||||||
| **connection_kwargs, | ||||||
| ): | ||||||
| max_connections = max_connections or 2**31 | ||||||
| if not isinstance(max_connections, int) or max_connections < 0: | ||||||
| raise ValueError('"max_connections" must be a positive integer') | ||||||
| if not isinstance(min_connections, int) or min_connections < 0: | ||||||
| raise ValueError('"min_connections" must be a non-negative integer') | ||||||
| if min_connections > max_connections: | ||||||
| raise ValueError( | ||||||
| '"min_connections" must be less than or equal to "max_connections"' | ||||||
| ) | ||||||
|
|
||||||
| self.connection_class = connection_class | ||||||
| self.connection_kwargs = connection_kwargs | ||||||
| self.max_connections = max_connections | ||||||
| self.min_connections = min_connections | ||||||
|
|
||||||
| self._available_connections: List[AbstractConnection] = [] | ||||||
| self._in_use_connections: Set[AbstractConnection] = set() | ||||||
| self.encoder_class = self.connection_kwargs.get("encoder_class", Encoder) | ||||||
|
|
||||||
| self._initialized = False | ||||||
|
|
||||||
| # Pre-create min_connections connection objects (connected lazily, | ||||||
| # or call initialize() to connect them eagerly) | ||||||
| for _ in range(self.min_connections): | ||||||
| self._available_connections.append(self.make_connection()) | ||||||
|
|
||||||
| async def initialize(self): | ||||||
| """Connect all pre-created connections from min_connections.""" | ||||||
| if self._initialized: | ||||||
| return | ||||||
|
Comment on lines
+1080
to
+1081
|
||||||
| if self._initialized: | |
| return |
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -204,6 +204,7 @@ def __init__( | |||||||||||||||||||||||||||||
| ssl_min_version=None, | ||||||||||||||||||||||||||||||
| ssl_ciphers=None, | ||||||||||||||||||||||||||||||
| max_connections=None, | ||||||||||||||||||||||||||||||
| min_connections=0, | ||||||||||||||||||||||||||||||
| single_connection_client=False, | ||||||||||||||||||||||||||||||
| health_check_interval=0, | ||||||||||||||||||||||||||||||
| client_name=None, | ||||||||||||||||||||||||||||||
|
|
@@ -231,6 +232,11 @@ def __init__( | |||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| Args: | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| max_connections: | ||||||||||||||||||||||||||||||
| The maximum number of connections for the pool. | ||||||||||||||||||||||||||||||
| min_connections: | ||||||||||||||||||||||||||||||
| The minimum number of connections to pre-create and connect | ||||||||||||||||||||||||||||||
| when the pool is initialized. Defaults to 0. | ||||||||||||||||||||||||||||||
| single_connection_client: | ||||||||||||||||||||||||||||||
| if `True`, connection pool is not used. In that case `Valkey` | ||||||||||||||||||||||||||||||
| instance use is not thread safe. | ||||||||||||||||||||||||||||||
|
Comment on lines
234
to
242
|
||||||||||||||||||||||||||||||
| max_connections: | |
| The maximum number of connections for the pool. | |
| min_connections: | |
| The minimum number of connections to pre-create and connect | |
| when the pool is initialized. Defaults to 0. | |
| single_connection_client: | |
| if `True`, connection pool is not used. In that case `Valkey` | |
| instance use is not thread safe. | |
| max_connections: The maximum number of connections for the pool. | |
| min_connections: The minimum number of connections to pre-create and | |
| connect when the pool is initialized. Defaults to 0. | |
| single_connection_client: if `True`, connection pool is not used. In | |
| that case `Valkey` instance use is not thread safe. |
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.
ClusterNode validates min_connections only against max_connections, but does not reject negative or non-int values. This can silently accept invalid input (e.g., min_connections=-1 results in no pre-created connections) and is inconsistent with ConnectionPool validation. Consider adding the same type/range checks here and raising ValkeyClusterException for invalid values.