From 1b5c0135bab619ee87a8f38215a8eb2907dd21f7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kristj=C3=A1n=20Valur=20J=C3=B3nsson?= Date: Sun, 20 Aug 2023 11:10:47 +0000 Subject: [PATCH 1/4] set the `auto_close_connection_pool` flag on Redis instance with its own pool. --- redis/asyncio/sentinel.py | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/redis/asyncio/sentinel.py b/redis/asyncio/sentinel.py index 501e234c3c..940382596a 100644 --- a/redis/asyncio/sentinel.py +++ b/redis/asyncio/sentinel.py @@ -335,10 +335,13 @@ def master_for( kwargs["is_master"] = True connection_kwargs = dict(self.connection_kwargs) connection_kwargs.update(kwargs) + + connection_pool = connection_pool_class(service_name, self, **connection_kwargs) + # The Redis object "owns" the pool + auto_close_connection_pool = True return redis_class( - connection_pool=connection_pool_class( - service_name, self, **connection_kwargs - ) + connection_pool=connection_pool, + auto_close_connection_pool=auto_close_connection_pool, ) def slave_for( @@ -368,8 +371,11 @@ def slave_for( kwargs["is_master"] = False connection_kwargs = dict(self.connection_kwargs) connection_kwargs.update(kwargs) + + connection_pool = connection_pool_class(service_name, self, **connection_kwargs) + # The Redis object "owns" the pool + auto_close_connection_pool = True return redis_class( - connection_pool=connection_pool_class( - service_name, self, **connection_kwargs - ) + connection_pool=connection_pool, + auto_close_connection_pool=auto_close_connection_pool, ) From 7e5e5fec7b73ca2d0ba9deb13b34c30a859d7c29 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kristj=C3=A1n=20Valur=20J=C3=B3nsson?= Date: Sun, 20 Aug 2023 11:34:33 +0000 Subject: [PATCH 2/4] Add tests for the Sentinel connection pool auto-close --- tests/test_asyncio/test_sentinel.py | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/tests/test_asyncio/test_sentinel.py b/tests/test_asyncio/test_sentinel.py index 2091f2cb87..c86d2641cd 100644 --- a/tests/test_asyncio/test_sentinel.py +++ b/tests/test_asyncio/test_sentinel.py @@ -1,4 +1,5 @@ import socket +from unittest import mock import pytest import pytest_asyncio @@ -239,3 +240,27 @@ async def test_flushconfig(cluster, sentinel): async def test_reset(cluster, sentinel): cluster.master["is_odown"] = True assert await sentinel.sentinel_reset("mymaster") + + +@pytest.mark.onlynoncluster +@pytest.mark.parametrize("method_name", ["master_for", "slave_for"]) +async def test_auto_close_pool(cluster, sentinel, method_name): + """ + Check that the connection pool created by the sentinel client is automatically closed + """ + + method = getattr(sentinel, method_name) + client = method("mymaster", db=9) + pool = client.connection_pool + assert client.auto_close_connection_pool is True + calls = 0 + + async def mock_disconnect(): + nonlocal calls + calls += 1 + + with mock.patch.object(pool, "disconnect", mock_disconnect) as disconnect: + await client.close() + + assert calls == 1 + await pool.disconnect() From 8de625e29e7d0f98081b6c266914ecac4e6f4245 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kristj=C3=A1n=20Valur=20J=C3=B3nsson?= Date: Sun, 20 Aug 2023 11:35:00 +0000 Subject: [PATCH 3/4] auto_close_connection_pool is ignored if a pool is provided --- redis/asyncio/sentinel.py | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/redis/asyncio/sentinel.py b/redis/asyncio/sentinel.py index 940382596a..bc3abe7580 100644 --- a/redis/asyncio/sentinel.py +++ b/redis/asyncio/sentinel.py @@ -339,10 +339,11 @@ def master_for( connection_pool = connection_pool_class(service_name, self, **connection_kwargs) # The Redis object "owns" the pool auto_close_connection_pool = True - return redis_class( + client = redis_class( connection_pool=connection_pool, - auto_close_connection_pool=auto_close_connection_pool, ) + client.auto_close_connection_pool = auto_close_connection_pool + return client def slave_for( self, @@ -375,7 +376,9 @@ def slave_for( connection_pool = connection_pool_class(service_name, self, **connection_kwargs) # The Redis object "owns" the pool auto_close_connection_pool = True - return redis_class( + client = redis_class( connection_pool=connection_pool, - auto_close_connection_pool=auto_close_connection_pool, ) + client.auto_close_connection_pool = auto_close_connection_pool + return client + From 2921cc87a6b33e986d4b56d36231e74568706933 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kristj=C3=A1n=20Valur=20J=C3=B3nsson?= Date: Tue, 22 Aug 2023 16:27:37 +0000 Subject: [PATCH 4/4] linting --- redis/asyncio/sentinel.py | 1 - tests/test_asyncio/test_sentinel.py | 5 +++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/redis/asyncio/sentinel.py b/redis/asyncio/sentinel.py index bc3abe7580..5ed924096c 100644 --- a/redis/asyncio/sentinel.py +++ b/redis/asyncio/sentinel.py @@ -381,4 +381,3 @@ def slave_for( ) client.auto_close_connection_pool = auto_close_connection_pool return client - diff --git a/tests/test_asyncio/test_sentinel.py b/tests/test_asyncio/test_sentinel.py index c86d2641cd..a2d52f17b7 100644 --- a/tests/test_asyncio/test_sentinel.py +++ b/tests/test_asyncio/test_sentinel.py @@ -246,7 +246,8 @@ async def test_reset(cluster, sentinel): @pytest.mark.parametrize("method_name", ["master_for", "slave_for"]) async def test_auto_close_pool(cluster, sentinel, method_name): """ - Check that the connection pool created by the sentinel client is automatically closed + Check that the connection pool created by the sentinel client is + automatically closed """ method = getattr(sentinel, method_name) @@ -259,7 +260,7 @@ async def mock_disconnect(): nonlocal calls calls += 1 - with mock.patch.object(pool, "disconnect", mock_disconnect) as disconnect: + with mock.patch.object(pool, "disconnect", mock_disconnect): await client.close() assert calls == 1