From 27e5e840c8f8389b8c2cb790a3a5ec6d8466c50c Mon Sep 17 00:00:00 2001 From: Samuel Guilhem-Ducleon Date: Tue, 9 Dec 2025 13:27:26 -0500 Subject: [PATCH 1/7] fix --- redis/asyncio/sentinel.py | 38 ++++++++++++++------------------------ 1 file changed, 14 insertions(+), 24 deletions(-) diff --git a/redis/asyncio/sentinel.py b/redis/asyncio/sentinel.py index d0455ab6eb..0507ebe4c8 100644 --- a/redis/asyncio/sentinel.py +++ b/redis/asyncio/sentinel.py @@ -39,31 +39,21 @@ def __repr__(self): s += host_info return s + ")>" - async def connect_to(self, address): - self.host, self.port = address - await self.connect_check_health( - check_health=self.connection_pool.check_connection, - retry_socket_connect=False, - ) - - async def _connect_retry(self): - if self._reader: - return # already connected + async def _connect(self): + if self.is_connected: + await super()._connect() + return None if self.connection_pool.is_master: - await self.connect_to(await self.connection_pool.get_master_address()) - else: - async for slave in self.connection_pool.rotate_slaves(): - try: - return await self.connect_to(slave) - except ConnectionError: - continue - raise SlaveNotFoundError # Never be here - - async def connect(self): - return await self.retry.call_with_retry( - self._connect_retry, - lambda error: asyncio.sleep(0), - ) + self.host, self.port = await self.connection_pool.get_master_address() + await super()._connect() + return None + async for slave in self.connection_pool.rotate_slaves(): + try: + self.host, self.port = slave + return await super()._connect() + except ConnectionError: + continue + raise SlaveNotFoundError # Never be here async def read_response( self, From 6a54da8809e87f2fbe940085158313c1283d4bc4 Mon Sep 17 00:00:00 2001 From: Samuel Guilhem-Ducleon Date: Tue, 9 Dec 2025 15:24:58 -0500 Subject: [PATCH 2/7] test --- .../test_sentinel_managed_connection.py | 31 ++++++++++--------- 1 file changed, 16 insertions(+), 15 deletions(-) diff --git a/tests/test_asyncio/test_sentinel_managed_connection.py b/tests/test_asyncio/test_sentinel_managed_connection.py index 5a511b2793..3f7f7df261 100644 --- a/tests/test_asyncio/test_sentinel_managed_connection.py +++ b/tests/test_asyncio/test_sentinel_managed_connection.py @@ -2,6 +2,8 @@ from unittest import mock import pytest + +from redis.asyncio import Connection from redis.asyncio.retry import Retry from redis.asyncio.sentinel import SentinelManagedConnection from redis.backoff import NoBackoff @@ -20,18 +22,17 @@ async def test_connect_retry_on_timeout_error(connect_args): retry=Retry(NoBackoff(), 3), connection_pool=connection_pool, ) - origin_connect = conn._connect - conn._connect = mock.AsyncMock() - - async def mock_connect(): - # connect only on the last retry - if conn._connect.call_count <= 2: - raise socket.timeout - else: - return await origin_connect() - - conn._connect.side_effect = mock_connect - await conn.connect() - assert conn._connect.call_count == 3 - assert connection_pool.get_master_address.call_count == 3 - await conn.disconnect() + original_super_connect = Connection._connect.__get__(conn, Connection) + + with mock.patch.object(Connection, "_connect", new_callable=mock.AsyncMock) as mock_super_connect: + async def side_effect(*args, **kwargs): + if mock_super_connect.await_count <= 2: + raise socket.timeout() + return await original_super_connect(*args, **kwargs) + + mock_super_connect.side_effect = side_effect + + await conn.connect() + assert mock_super_connect.await_count == 3 + assert connection_pool.get_master_address.call_count == 3 + await conn.disconnect() \ No newline at end of file From 543a437a8f76a147227dda7592981ea62d07fae0 Mon Sep 17 00:00:00 2001 From: Samuel Guilhem-Ducleon Date: Tue, 9 Dec 2025 15:32:44 -0500 Subject: [PATCH 3/7] fix --- redis/asyncio/connection.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/redis/asyncio/connection.py b/redis/asyncio/connection.py index 1d50b53ee2..72f5d75dac 100644 --- a/redis/asyncio/connection.py +++ b/redis/asyncio/connection.py @@ -323,6 +323,8 @@ async def connect_check_health( raise TimeoutError("Timeout connecting to server") except OSError as e: raise ConnectionError(self._error_message(e)) + except ConnectionError: + raise except Exception as exc: raise ConnectionError(exc) from exc From 330d2dd35f4af79eecbc3f590e566f775a600b3c Mon Sep 17 00:00:00 2001 From: Samuel Guilhem-Ducleon Date: Tue, 9 Dec 2025 15:41:34 -0500 Subject: [PATCH 4/7] add test --- .../test_sentinel_managed_connection.py | 26 +++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/tests/test_asyncio/test_sentinel_managed_connection.py b/tests/test_asyncio/test_sentinel_managed_connection.py index 3f7f7df261..e4d11457a3 100644 --- a/tests/test_asyncio/test_sentinel_managed_connection.py +++ b/tests/test_asyncio/test_sentinel_managed_connection.py @@ -35,4 +35,30 @@ async def side_effect(*args, **kwargs): await conn.connect() assert mock_super_connect.await_count == 3 assert connection_pool.get_master_address.call_count == 3 + await conn.disconnect() + +async def test_connect_check_health_retry_on_timeout_error(connect_args): + """Test that the _connect function is retried in case of a timeout""" + connection_pool = mock.AsyncMock() + connection_pool.get_master_address = mock.AsyncMock( + return_value=(connect_args["host"], connect_args["port"]) + ) + conn = SentinelManagedConnection( + retry_on_timeout=True, + retry=Retry(NoBackoff(), 3), + connection_pool=connection_pool, + ) + original_super_connect = Connection._connect.__get__(conn, Connection) + + with mock.patch.object(Connection, "_connect", new_callable=mock.AsyncMock) as mock_super_connect: + async def side_effect(*args, **kwargs): + if mock_super_connect.await_count <= 2: + raise socket.timeout() + return await original_super_connect(*args, **kwargs) + + mock_super_connect.side_effect = side_effect + + await conn.connect_check_health() + assert mock_super_connect.await_count == 3 + assert connection_pool.get_master_address.call_count == 3 await conn.disconnect() \ No newline at end of file From 98fe4385c7e78b4bbca4a4cba5ac69978e7ea90f Mon Sep 17 00:00:00 2001 From: Samuel Guilhem-Ducleon Date: Tue, 9 Dec 2025 16:27:11 -0500 Subject: [PATCH 5/7] sync client --- redis/sentinel.py | 34 +++++++++++++--------------------- 1 file changed, 13 insertions(+), 21 deletions(-) diff --git a/redis/sentinel.py b/redis/sentinel.py index f12bd8dd5d..4a5bc12d13 100644 --- a/redis/sentinel.py +++ b/redis/sentinel.py @@ -37,29 +37,21 @@ def __repr__(self): s = s % host_info return s - def connect_to(self, address): - self.host, self.port = address - - self.connect_check_health( - check_health=self.connection_pool.check_connection, - retry_socket_connect=False, - ) - - def _connect_retry(self): + def _connect(self): if self._sock: - return # already connected + super()._connect() + return None # already connected if self.connection_pool.is_master: - self.connect_to(self.connection_pool.get_master_address()) - else: - for slave in self.connection_pool.rotate_slaves(): - try: - return self.connect_to(slave) - except ConnectionError: - continue - raise SlaveNotFoundError # Never be here - - def connect(self): - return self.retry.call_with_retry(self._connect_retry, lambda error: None) + self.host, self.port = self.connection_pool.get_master_address() + super()._connect() + return None + for slave in self.connection_pool.rotate_slaves(): + try: + self.host, self.port = slave + return super()._connect() + except ConnectionError: + continue + raise SlaveNotFoundError # Never be here def read_response( self, From cbe0ff782fe8c2fac12be20ec430a025c3976aa9 Mon Sep 17 00:00:00 2001 From: Samuel Guilhem-Ducleon Date: Tue, 9 Dec 2025 16:36:30 -0500 Subject: [PATCH 6/7] sync client --- redis/asyncio/sentinel.py | 3 +- redis/sentinel.py | 6 +-- tests/test_sentinel_managed_connection.py | 58 ++++++++++++++++------- 3 files changed, 46 insertions(+), 21 deletions(-) diff --git a/redis/asyncio/sentinel.py b/redis/asyncio/sentinel.py index 0507ebe4c8..9d8c170556 100644 --- a/redis/asyncio/sentinel.py +++ b/redis/asyncio/sentinel.py @@ -50,7 +50,8 @@ async def _connect(self): async for slave in self.connection_pool.rotate_slaves(): try: self.host, self.port = slave - return await super()._connect() + await super()._connect() + return None except ConnectionError: continue raise SlaveNotFoundError # Never be here diff --git a/redis/sentinel.py b/redis/sentinel.py index 4a5bc12d13..8f14f2a752 100644 --- a/redis/sentinel.py +++ b/redis/sentinel.py @@ -39,12 +39,10 @@ def __repr__(self): def _connect(self): if self._sock: - super()._connect() - return None # already connected + return super()._connect() # already connected if self.connection_pool.is_master: self.host, self.port = self.connection_pool.get_master_address() - super()._connect() - return None + return super()._connect() for slave in self.connection_pool.rotate_slaves(): try: self.host, self.port = slave diff --git a/tests/test_sentinel_managed_connection.py b/tests/test_sentinel_managed_connection.py index 6fe5f7cd5b..666f0346d2 100644 --- a/tests/test_sentinel_managed_connection.py +++ b/tests/test_sentinel_managed_connection.py @@ -1,9 +1,10 @@ import socket +from unittest import mock +from redis import Connection from redis.retry import Retry from redis.sentinel import SentinelManagedConnection from redis.backoff import NoBackoff -from unittest import mock def test_connect_retry_on_timeout_error(master_host): @@ -17,18 +18,43 @@ def test_connect_retry_on_timeout_error(master_host): retry=Retry(NoBackoff(), 3), connection_pool=connection_pool, ) - origin_connect = conn._connect - conn._connect = mock.Mock() - - def mock_connect(): - # connect only on the last retry - if conn._connect.call_count <= 2: - raise socket.timeout - else: - return origin_connect() - - conn._connect.side_effect = mock_connect - conn.connect() - assert conn._connect.call_count == 3 - assert connection_pool.get_master_address.call_count == 3 - conn.disconnect() + original_super_connect = Connection._connect.__get__(conn, Connection) + + with mock.patch.object(Connection, "_connect", new_callable=mock.Mock) as mock_super_connect: + def side_effect(*args, **kwargs): + if mock_super_connect.call_count <= 2: + raise socket.timeout() + return original_super_connect(*args, **kwargs) + + mock_super_connect.side_effect = side_effect + + conn.connect() + assert mock_super_connect.call_count == 3 + assert connection_pool.get_master_address.call_count == 3 + conn.disconnect() + +def test_connect_check_health_retry_on_timeout_error(master_host): + """Test that the _connect function is retried in case of a timeout""" + connection_pool = mock.Mock() + connection_pool.get_master_address = mock.Mock( + return_value=(master_host[0], master_host[1]) + ) + conn = SentinelManagedConnection( + retry_on_timeout=True, + retry=Retry(NoBackoff(), 3), + connection_pool=connection_pool, + ) + original_super_connect = Connection._connect.__get__(conn, Connection) + + with mock.patch.object(Connection, "_connect", new_callable=mock.Mock) as mock_super_connect: + def side_effect(*args, **kwargs): + if mock_super_connect.call_count <= 2: + raise socket.timeout() + return original_super_connect(*args, **kwargs) + + mock_super_connect.side_effect = side_effect + + conn.connect_check_health() + assert mock_super_connect.call_count == 3 + assert connection_pool.get_master_address.call_count == 3 + conn.disconnect() \ No newline at end of file From ca6ecd0923ff7e282c431c8692d8a561fe636769 Mon Sep 17 00:00:00 2001 From: Samuel Guilhem-Ducleon Date: Wed, 10 Dec 2025 11:33:47 -0500 Subject: [PATCH 7/7] lint --- redis/sentinel.py | 2 +- .../test_sentinel_managed_connection.py | 13 ++++++++++--- tests/test_sentinel_managed_connection.py | 13 ++++++++++--- 3 files changed, 21 insertions(+), 7 deletions(-) diff --git a/redis/sentinel.py b/redis/sentinel.py index 8f14f2a752..edc47b5540 100644 --- a/redis/sentinel.py +++ b/redis/sentinel.py @@ -39,7 +39,7 @@ def __repr__(self): def _connect(self): if self._sock: - return super()._connect() # already connected + return super()._connect() # already connected if self.connection_pool.is_master: self.host, self.port = self.connection_pool.get_master_address() return super()._connect() diff --git a/tests/test_asyncio/test_sentinel_managed_connection.py b/tests/test_asyncio/test_sentinel_managed_connection.py index e4d11457a3..cec55cdc1c 100644 --- a/tests/test_asyncio/test_sentinel_managed_connection.py +++ b/tests/test_asyncio/test_sentinel_managed_connection.py @@ -24,7 +24,10 @@ async def test_connect_retry_on_timeout_error(connect_args): ) original_super_connect = Connection._connect.__get__(conn, Connection) - with mock.patch.object(Connection, "_connect", new_callable=mock.AsyncMock) as mock_super_connect: + with mock.patch.object( + Connection, "_connect", new_callable=mock.AsyncMock + ) as mock_super_connect: + async def side_effect(*args, **kwargs): if mock_super_connect.await_count <= 2: raise socket.timeout() @@ -37,6 +40,7 @@ async def side_effect(*args, **kwargs): assert connection_pool.get_master_address.call_count == 3 await conn.disconnect() + async def test_connect_check_health_retry_on_timeout_error(connect_args): """Test that the _connect function is retried in case of a timeout""" connection_pool = mock.AsyncMock() @@ -50,7 +54,10 @@ async def test_connect_check_health_retry_on_timeout_error(connect_args): ) original_super_connect = Connection._connect.__get__(conn, Connection) - with mock.patch.object(Connection, "_connect", new_callable=mock.AsyncMock) as mock_super_connect: + with mock.patch.object( + Connection, "_connect", new_callable=mock.AsyncMock + ) as mock_super_connect: + async def side_effect(*args, **kwargs): if mock_super_connect.await_count <= 2: raise socket.timeout() @@ -61,4 +68,4 @@ async def side_effect(*args, **kwargs): await conn.connect_check_health() assert mock_super_connect.await_count == 3 assert connection_pool.get_master_address.call_count == 3 - await conn.disconnect() \ No newline at end of file + await conn.disconnect() diff --git a/tests/test_sentinel_managed_connection.py b/tests/test_sentinel_managed_connection.py index 666f0346d2..6ac16c76dd 100644 --- a/tests/test_sentinel_managed_connection.py +++ b/tests/test_sentinel_managed_connection.py @@ -20,7 +20,10 @@ def test_connect_retry_on_timeout_error(master_host): ) original_super_connect = Connection._connect.__get__(conn, Connection) - with mock.patch.object(Connection, "_connect", new_callable=mock.Mock) as mock_super_connect: + with mock.patch.object( + Connection, "_connect", new_callable=mock.Mock + ) as mock_super_connect: + def side_effect(*args, **kwargs): if mock_super_connect.call_count <= 2: raise socket.timeout() @@ -33,6 +36,7 @@ def side_effect(*args, **kwargs): assert connection_pool.get_master_address.call_count == 3 conn.disconnect() + def test_connect_check_health_retry_on_timeout_error(master_host): """Test that the _connect function is retried in case of a timeout""" connection_pool = mock.Mock() @@ -46,7 +50,10 @@ def test_connect_check_health_retry_on_timeout_error(master_host): ) original_super_connect = Connection._connect.__get__(conn, Connection) - with mock.patch.object(Connection, "_connect", new_callable=mock.Mock) as mock_super_connect: + with mock.patch.object( + Connection, "_connect", new_callable=mock.Mock + ) as mock_super_connect: + def side_effect(*args, **kwargs): if mock_super_connect.call_count <= 2: raise socket.timeout() @@ -57,4 +64,4 @@ def side_effect(*args, **kwargs): conn.connect_check_health() assert mock_super_connect.call_count == 3 assert connection_pool.get_master_address.call_count == 3 - conn.disconnect() \ No newline at end of file + conn.disconnect()