use from_pool() class method instead

Author: kristjanvalur

docs/examples/asyncio_examples.ipynb

@@ -48,7 +48,7 @@
    "cell_type": "markdown",
    "metadata": {},
    "source": [
-    "If you create custom `ConnectionPool` for the `Redis` instance to use, use the `from_pool` argument.  This will cause the pool to be disconnected along with the Redis instance. Disconnecting the connection pool simply disconnects all connections hosted in the pool."
+    "If you create custom `ConnectionPool` for the `Redis` instance to use alone, use the `from_pool` class method to create it.  This will cause the pool to be disconnected along with the Redis instance. Disconnecting the connection pool simply disconnects all connections hosted in the pool."
    ]
   },
   {
@@ -60,7 +60,7 @@
     "import redis.asyncio as redis\n",
     "\n",
     "pool = redis.ConnectionPool.from_url(\"redis://localhost\")\n",
-    "connection = redis.Redis(from_pool=pool)\n",
+    "connection = redis.Redis.from_pool(pool)\n",
     "await connection.close()"
    ]
   },

redis/asyncio/client.py

@@ -159,10 +159,28 @@ class initializer. In the case of conflicting arguments, querystring
 
         """
         connection_pool = ConnectionPool.from_url(url, **kwargs)
-        return cls(
-            from_pool=connection_pool,
+        client = cls(
+            connection_pool=connection_pool,
             single_connection_client=single_connection_client,
         )
+        client.auto_close_connection_pool = True
+        return client
+
+    @classmethod
+    def from_pool(
+        cls: Type["Redis"],
+        connection_pool: ConnectionPool,
+    ) -> "Redis":
+        """
+        Return a Redis client from teh given connection pool.
+        The Redis client will take ownership of the connection pool and
+        close it when the Redis client is closed.
+        """
+        client = cls(
+            connection_pool=connection_pool,
+        )
+        client.auto_close_connection_pool = True
+        return client
 
     def __init__(
         self,
@@ -176,7 +194,6 @@ def __init__(
         socket_keepalive: Optional[bool] = None,
         socket_keepalive_options: Optional[Mapping[int, Union[int, bytes]]] = None,
         connection_pool: Optional[ConnectionPool] = None,
-        from_pool: Optional[ConnectionPool] = None,
         unix_socket_path: Optional[str] = None,
         encoding: str = "utf-8",
         encoding_errors: str = "strict",
@@ -213,7 +230,7 @@ def __init__(
         """
         kwargs: Dict[str, Any]
 
-        if not connection_pool and not from_pool:
+        if not connection_pool:
             # Create internal connection pool, expected to be closed by Redis instance
             if not retry_on_error:
                 retry_on_error = []
@@ -271,28 +288,14 @@ def __init__(
                             "ssl_check_hostname": ssl_check_hostname,
                         }
                     )
-            # backwards compatibility.  This arg only used if no pool
-            # is provided
-            if auto_close_connection_pool:
-                from_pool = ConnectionPool(**kwargs)
-            else:
-                connection_pool = ConnectionPool(**kwargs)
-
-        if from_pool is not None:
-            # internal connection pool, expected to be closed by Redis instance
-            if connection_pool is not None:
-                raise ValueError(
-                    "Cannot use both from_pool and connection_pool arguments"
-                )
-            self.connection_pool = from_pool
-            self.auto_close_connection_pool = True  # the Redis instance closes the pool
+            # This arg only used if no pool is passed in
+            self.auto_close_connection_pool = auto_close_connection_pool
+            connection_pool = ConnectionPool(**kwargs)
         else:
-            # external connection pool, expected to be closed by caller
-            self.connection_pool = connection_pool
-            self.auto_close_connection_pool = (
-                False  # the user is expected to close the pool
-            )
+            # If a pool is passed in, do not close it
+            self.auto_close_connection_pool = False
 
+        self.connection_pool = connection_pool
         self.single_connection_client = single_connection_client
         self.connection: Optional[Connection] = None
 

redis/asyncio/connection.py

@@ -1198,8 +1198,8 @@ class BlockingConnectionPool(ConnectionPool):
     """
     Thread-safe blocking connection pool::
 
-        >>> from redis.client import Redis
-        >>> client = Redis(from_pool=BlockingConnectionPool())
+        >>> from redis.asyncio import Redis, BlockingConnectionPool
+        >>> client = Redis.from_pool(BlockingConnectionPool())
 
     It performs the same function as the default
     :py:class:`~redis.ConnectionPool` implementation, in that,

redis/asyncio/sentinel.py

@@ -338,9 +338,7 @@ def master_for(
 
         connection_pool = connection_pool_class(service_name, self, **connection_kwargs)
         # The Redis object "owns" the pool
-        return redis_class(
-            from_pool=connection_pool,
-        )
+        return redis_class.from_pool(connection_pool)
 
     def slave_for(
         self,
@@ -372,6 +370,4 @@ def slave_for(
 
         connection_pool = connection_pool_class(service_name, self, **connection_kwargs)
         # The Redis object "owns" the pool
-        return redis_class(
-            from_pool=connection_pool,
-        )
+        return redis_class.from_pool(connection_pool)

redis/client.py

@@ -4,7 +4,7 @@
 import time
 import warnings
 from itertools import chain
-from typing import Optional
+from typing import Optional, Type
 
 from redis._parsers.helpers import (
     _RedisCallbacks,
@@ -136,10 +136,28 @@ class initializer. In the case of conflicting arguments, querystring
         """
         single_connection_client = kwargs.pop("single_connection_client", False)
         connection_pool = ConnectionPool.from_url(url, **kwargs)
-        return cls(
-            from_pool=connection_pool,
+        client = cls(
+            connection_pool=connection_pool,
             single_connection_client=single_connection_client,
         )
+        client.auto_close_connection_pool = True
+        return client
+
+    @classmethod
+    def from_pool(
+        cls: Type["Redis"],
+        connection_pool: ConnectionPool,
+    ) -> "Redis":
+        """
+        Return a Redis client from teh given connection pool.
+        The Redis client will take ownership of the connection pool and
+        close it when the Redis client is closed.
+        """
+        client = cls(
+            connection_pool=connection_pool,
+        )
+        client.auto_close_connection_pool = True
+        return client
 
     def __init__(
         self,
@@ -152,7 +170,6 @@ def __init__(
         socket_keepalive=None,
         socket_keepalive_options=None,
         connection_pool=None,
-        from_pool=None,
         unix_socket_path=None,
         encoding="utf-8",
         encoding_errors="strict",
@@ -199,7 +216,7 @@ def __init__(
             if `True`, connection pool is not used. In that case `Redis`
             instance use is not thread safe.
         """
-        if not connection_pool and not from_pool:
+        if not connection_pool:
             if charset is not None:
                 warnings.warn(
                     DeprecationWarning(
@@ -275,20 +292,12 @@ def __init__(
                             "ssl_ocsp_expected_cert": ssl_ocsp_expected_cert,
                         }
                     )
-            from_pool = ConnectionPool(**kwargs)
-        
-        if from_pool is not None:
-            # internal connection pool, expected to be closed by Redis instance
-            if connection_pool is not None:
-                raise ValueError(
-                    "Cannot use both from_pool and connection_pool arguments")
-            self.connection_pool = from_pool
-            self.auto_close_connection_pool = True  # the Redis instance closes the pool
+            connection_pool = ConnectionPool(**kwargs)
+            self.auto_close_connection_pool = True
         else:
-            # external connection pool, expected to be closed by caller
-            self.connection_pool = connection_pool
-            self.auto_close_connection_pool = False  # the user is expected to close the pool
+            self.auto_close_connection_pool = False
 
+        self.connection_pool = connection_pool
         self.connection = None
         if single_connection_client:
             self.connection = self.connection_pool.get_connection("_")

redis/sentinel.py

@@ -348,8 +348,8 @@ def master_for(
         kwargs["is_master"] = True
         connection_kwargs = dict(self.connection_kwargs)
         connection_kwargs.update(kwargs)
-        return redis_class(
-            from_pool=connection_pool_class(service_name, self, **connection_kwargs)
+        return redis_class.from_pool(
+            connection_pool_class(service_name, self, **connection_kwargs)
         )
 
     def slave_for(
@@ -379,6 +379,6 @@ def slave_for(
         kwargs["is_master"] = False
         connection_kwargs = dict(self.connection_kwargs)
         connection_kwargs.update(kwargs)
-        return redis_class(
-            from_pool=connection_pool_class(service_name, self, **connection_kwargs)
+        return redis_class.from_pool(
+            connection_pool_class(service_name, self, **connection_kwargs)
         )

tests/test_asyncio/test_connection.py

@@ -356,7 +356,7 @@ async def mock_disconnect(_):
 
 @pytest.mark.parametrize("from_url", (True, False), ids=("from_url", "from_args"))
 async def test_redis_from_pool(request, from_url):
-    """Verify that basic Redis instances using `from_pool`
+    """Verify that basic Redis instances created using `from_pool()`
     have auto_close_connection_pool set to True"""
 
     url: str = request.config.getoption("--redis-url")
@@ -370,7 +370,7 @@ async def get_redis_connection():
             pool = ConnectionPool.from_url(url)
         else:
             pool = ConnectionPool(**url_args)
-        return Redis(from_pool=pool)
+        return Redis.from_pool(pool)
 
     called = 0
 
@@ -384,3 +384,29 @@ async def mock_disconnect(_):
 
     assert called == 1
     await pool.disconnect()
+
+
+@pytest.mark.parametrize("auto_close", (True, False))
+async def test_redis_pool_auto_close_arg(request, auto_close):
+    """test that redis instance where pool is provided have
+    auto_close_connection_pool set to False, regardless of arg"""
+
+    url: str = request.config.getoption("--redis-url")
+    pool = ConnectionPool.from_url(url)
+
+    async def get_redis_connection():
+        client = Redis(connection_pool=pool, auto_close_connection_pool=auto_close)
+        return client
+
+    called = 0
+
+    async def mock_disconnect(_):
+        nonlocal called
+        called += 1
+
+    with patch.object(ConnectionPool, "disconnect", mock_disconnect):
+        async with await get_redis_connection() as r1:
+            assert r1.auto_close_connection_pool is False
+
+    assert called == 0
+    await pool.disconnect()

tests/test_connection.py

@@ -268,7 +268,7 @@ def mock_disconnect(_):
 
 @pytest.mark.parametrize("from_url", (True, False), ids=("from_url", "from_args"))
 def test_redis_from_pool(request, from_url):
-    """Verify that basic Redis instances using `from_pool`
+    """Verify that basic Redis instances created using `from_pool()`
     have auto_close_connection_pool set to True"""
 
     url: str = request.config.getoption("--redis-url")
@@ -282,7 +282,7 @@ def get_redis_connection():
             pool = ConnectionPool.from_url(url)
         else:
             pool = ConnectionPool(**url_args)
-        return Redis(from_pool=pool)
+        return Redis.from_pool(pool)
 
     called = 0