From 2746dcd69956a0baa4ed9dc2ddd0826408181ea7 Mon Sep 17 00:00:00 2001 From: mahigupta Date: Sat, 23 Mar 2024 13:43:50 -0700 Subject: [PATCH 1/7] Fixing sentinel command response --- redis/asyncio/sentinel.py | 26 ++++++++++++++++++-------- redis/commands/sentinel.py | 20 ++++++++++---------- redis/sentinel.py | 21 ++++++++++++++++----- 3 files changed, 44 insertions(+), 23 deletions(-) diff --git a/redis/asyncio/sentinel.py b/redis/asyncio/sentinel.py index d88babc59c..1f0c0fd253 100644 --- a/redis/asyncio/sentinel.py +++ b/redis/asyncio/sentinel.py @@ -1,6 +1,7 @@ import asyncio import random import weakref +from functools import reduce from typing import AsyncIterator, Iterable, Mapping, Optional, Sequence, Tuple, Type from redis.asyncio.client import Redis @@ -227,16 +228,25 @@ async def execute_command(self, *args, **kwargs): once = bool(kwargs.get("once", False)) if "once" in kwargs.keys(): kwargs.pop("once") + + # Check if command suppose to return boolean response. + bool_resp = bool(kwargs.get("bool_resp", False)) + if "bool_resp" in kwargs.keys(): + kwargs.pop("bool_resp") if once: - await random.choice(self.sentinels).execute_command(*args, **kwargs) - else: - tasks = [ - asyncio.Task(sentinel.execute_command(*args, **kwargs)) - for sentinel in self.sentinels - ] - await asyncio.gather(*tasks) - return True + return await random.choice(self.sentinels).execute_command(*args, **kwargs) + + tasks = [ + asyncio.Task(sentinel.execute_command(*args, **kwargs)) + for sentinel in self.sentinels + ] + responses = await asyncio.gather(*tasks) + + if bool_resp: + return reduce(lambda x, y: x and y, responses) + + return responses def __repr__(self): sentinel_addresses = [] diff --git a/redis/commands/sentinel.py b/redis/commands/sentinel.py index f745757955..eb7f08c448 100644 --- a/redis/commands/sentinel.py +++ b/redis/commands/sentinel.py @@ -13,7 +13,7 @@ def sentinel(self, *args): def sentinel_get_master_addr_by_name(self, service_name): """Returns a (host, port) pair for the given ``service_name``""" - return self.execute_command("SENTINEL GET-MASTER-ADDR-BY-NAME", service_name) + return self.execute_command("SENTINEL GET-MASTER-ADDR-BY-NAME", service_name, once=True) def sentinel_master(self, service_name): """Returns a dictionary containing the specified masters state.""" @@ -21,15 +21,15 @@ def sentinel_master(self, service_name): def sentinel_masters(self): """Returns a list of dictionaries containing each master's state.""" - return self.execute_command("SENTINEL MASTERS") + return self.execute_command("SENTINEL MASTERS", once=True) def sentinel_monitor(self, name, ip, port, quorum): """Add a new master to Sentinel to be monitored""" - return self.execute_command("SENTINEL MONITOR", name, ip, port, quorum) + return self.execute_command("SENTINEL MONITOR", name, ip, port, quorum, bool_resp=True) def sentinel_remove(self, name): """Remove a master from Sentinel's monitoring""" - return self.execute_command("SENTINEL REMOVE", name) + return self.execute_command("SENTINEL REMOVE", name, bool_resp=True) def sentinel_sentinels(self, service_name): """Returns a list of sentinels for ``service_name``""" @@ -37,11 +37,11 @@ def sentinel_sentinels(self, service_name): def sentinel_set(self, name, option, value): """Set Sentinel monitoring parameters for a given master""" - return self.execute_command("SENTINEL SET", name, option, value) + return self.execute_command("SENTINEL SET", name, option, value, bool_resp=True) def sentinel_slaves(self, service_name): """Returns a list of slaves for ``service_name``""" - return self.execute_command("SENTINEL SLAVES", service_name) + return self.execute_command("SENTINEL SLAVES", service_name, once=True) def sentinel_reset(self, pattern): """ @@ -52,7 +52,7 @@ def sentinel_reset(self, pattern): failover in progress), and removes every slave and sentinel already discovered and associated with the master. """ - return self.execute_command("SENTINEL RESET", pattern, once=True) + return self.execute_command("SENTINEL RESET", pattern, once=True, bool_resp=True) def sentinel_failover(self, new_master_name): """ @@ -61,7 +61,7 @@ def sentinel_failover(self, new_master_name): configuration will be published so that the other Sentinels will update their configurations). """ - return self.execute_command("SENTINEL FAILOVER", new_master_name) + return self.execute_command("SENTINEL FAILOVER", new_master_name, bool_resp=True) def sentinel_ckquorum(self, new_master_name): """ @@ -72,7 +72,7 @@ def sentinel_ckquorum(self, new_master_name): This command should be used in monitoring systems to check if a Sentinel deployment is ok. """ - return self.execute_command("SENTINEL CKQUORUM", new_master_name, once=True) + return self.execute_command("SENTINEL CKQUORUM", new_master_name, once=True, bool_resp=True) def sentinel_flushconfig(self): """ @@ -90,7 +90,7 @@ def sentinel_flushconfig(self): This command works even if the previous configuration file is completely missing. """ - return self.execute_command("SENTINEL FLUSHCONFIG") + return self.execute_command("SENTINEL FLUSHCONFIG", bool_resp=True) class AsyncSentinelCommands(SentinelCommands): diff --git a/redis/sentinel.py b/redis/sentinel.py index dfcd8ff64b..611962b328 100644 --- a/redis/sentinel.py +++ b/redis/sentinel.py @@ -1,5 +1,6 @@ import random import weakref +from functools import reduce from typing import Optional from redis.client import Redis @@ -254,13 +255,23 @@ def execute_command(self, *args, **kwargs): once = bool(kwargs.get("once", False)) if "once" in kwargs.keys(): kwargs.pop("once") + + # Check if command suppose to return boolean response. + bool_resp = bool(kwargs.get("bool_resp", False)) + if "bool_resp" in kwargs.keys(): + kwargs.pop("bool_resp") if once: - random.choice(self.sentinels).execute_command(*args, **kwargs) - else: - for sentinel in self.sentinels: - sentinel.execute_command(*args, **kwargs) - return True + return random.choice(self.sentinels).execute_command(*args, **kwargs) + + responses = [] + for sentinel in self.sentinels: + responses.append(sentinel.execute_command(*args, **kwargs)) + + if bool_resp: + return reduce(lambda x, y: x and y, responses) + + return responses def __repr__(self): sentinel_addresses = [] From c2d207b13d7a8a22581afecfabaa90e63c38d762 Mon Sep 17 00:00:00 2001 From: mahigupta Date: Sat, 23 Mar 2024 14:00:48 -0700 Subject: [PATCH 2/7] Linter check pass --- CHANGES | 1 + redis/asyncio/sentinel.py | 6 +++--- redis/commands/sentinel.py | 20 +++++++++++++++----- redis/sentinel.py | 6 +++--- 4 files changed, 22 insertions(+), 11 deletions(-) diff --git a/CHANGES b/CHANGES index e0959b0ef3..bbeb1aba35 100644 --- a/CHANGES +++ b/CHANGES @@ -1,3 +1,4 @@ + * Fixed sentinel execute command response * Allow to control the minimum SSL version * Add an optional lock_name attribute to LockError. * Fix return types for `get`, `set_path` and `strappend` in JSONCommands diff --git a/redis/asyncio/sentinel.py b/redis/asyncio/sentinel.py index 1f0c0fd253..2696b15344 100644 --- a/redis/asyncio/sentinel.py +++ b/redis/asyncio/sentinel.py @@ -228,7 +228,7 @@ async def execute_command(self, *args, **kwargs): once = bool(kwargs.get("once", False)) if "once" in kwargs.keys(): kwargs.pop("once") - + # Check if command suppose to return boolean response. bool_resp = bool(kwargs.get("bool_resp", False)) if "bool_resp" in kwargs.keys(): @@ -236,7 +236,7 @@ async def execute_command(self, *args, **kwargs): if once: return await random.choice(self.sentinels).execute_command(*args, **kwargs) - + tasks = [ asyncio.Task(sentinel.execute_command(*args, **kwargs)) for sentinel in self.sentinels @@ -245,7 +245,7 @@ async def execute_command(self, *args, **kwargs): if bool_resp: return reduce(lambda x, y: x and y, responses) - + return responses def __repr__(self): diff --git a/redis/commands/sentinel.py b/redis/commands/sentinel.py index eb7f08c448..382f9c9b12 100644 --- a/redis/commands/sentinel.py +++ b/redis/commands/sentinel.py @@ -13,7 +13,9 @@ def sentinel(self, *args): def sentinel_get_master_addr_by_name(self, service_name): """Returns a (host, port) pair for the given ``service_name``""" - return self.execute_command("SENTINEL GET-MASTER-ADDR-BY-NAME", service_name, once=True) + return self.execute_command( + "SENTINEL GET-MASTER-ADDR-BY-NAME", service_name, once=True + ) def sentinel_master(self, service_name): """Returns a dictionary containing the specified masters state.""" @@ -25,7 +27,9 @@ def sentinel_masters(self): def sentinel_monitor(self, name, ip, port, quorum): """Add a new master to Sentinel to be monitored""" - return self.execute_command("SENTINEL MONITOR", name, ip, port, quorum, bool_resp=True) + return self.execute_command( + "SENTINEL MONITOR", name, ip, port, quorum, bool_resp=True + ) def sentinel_remove(self, name): """Remove a master from Sentinel's monitoring""" @@ -52,7 +56,9 @@ def sentinel_reset(self, pattern): failover in progress), and removes every slave and sentinel already discovered and associated with the master. """ - return self.execute_command("SENTINEL RESET", pattern, once=True, bool_resp=True) + return self.execute_command( + "SENTINEL RESET", pattern, once=True, bool_resp=True + ) def sentinel_failover(self, new_master_name): """ @@ -61,7 +67,9 @@ def sentinel_failover(self, new_master_name): configuration will be published so that the other Sentinels will update their configurations). """ - return self.execute_command("SENTINEL FAILOVER", new_master_name, bool_resp=True) + return self.execute_command( + "SENTINEL FAILOVER", new_master_name, bool_resp=True + ) def sentinel_ckquorum(self, new_master_name): """ @@ -72,7 +80,9 @@ def sentinel_ckquorum(self, new_master_name): This command should be used in monitoring systems to check if a Sentinel deployment is ok. """ - return self.execute_command("SENTINEL CKQUORUM", new_master_name, once=True, bool_resp=True) + return self.execute_command( + "SENTINEL CKQUORUM", new_master_name, once=True, bool_resp=True + ) def sentinel_flushconfig(self): """ diff --git a/redis/sentinel.py b/redis/sentinel.py index 611962b328..220b32ed58 100644 --- a/redis/sentinel.py +++ b/redis/sentinel.py @@ -255,7 +255,7 @@ def execute_command(self, *args, **kwargs): once = bool(kwargs.get("once", False)) if "once" in kwargs.keys(): kwargs.pop("once") - + # Check if command suppose to return boolean response. bool_resp = bool(kwargs.get("bool_resp", False)) if "bool_resp" in kwargs.keys(): @@ -267,10 +267,10 @@ def execute_command(self, *args, **kwargs): responses = [] for sentinel in self.sentinels: responses.append(sentinel.execute_command(*args, **kwargs)) - + if bool_resp: return reduce(lambda x, y: x and y, responses) - + return responses def __repr__(self): From 1ef7f342094998b2ccffe51b6805a0bde157c17c Mon Sep 17 00:00:00 2001 From: Petya Slavova Date: Thu, 15 May 2025 19:03:43 +0300 Subject: [PATCH 3/7] Applying review comments and fixing some issues. --- redis/asyncio/sentinel.py | 25 +++++++++-------- redis/commands/sentinel.py | 55 +++++++++++++++++++++++--------------- redis/sentinel.py | 23 ++++++++-------- 3 files changed, 59 insertions(+), 44 deletions(-) diff --git a/redis/asyncio/sentinel.py b/redis/asyncio/sentinel.py index ba9aff6b46..821034bb3b 100644 --- a/redis/asyncio/sentinel.py +++ b/redis/asyncio/sentinel.py @@ -224,17 +224,20 @@ async def execute_command(self, *args, **kwargs): once - If set to True, then execute the resulting command on a single node at random, rather than across the entire sentinel cluster. """ - once = bool(kwargs.get("once", False)) - if "once" in kwargs.keys(): - kwargs.pop("once") + once = bool(kwargs.pop("once", False)) - # Check if command suppose to return boolean response. - bool_resp = bool(kwargs.get("bool_resp", False)) - if "bool_resp" in kwargs.keys(): - kwargs.pop("bool_resp") + # Check if command is supposed to return the original + # responces instead of boolean value. + return_responses = bool(kwargs.pop("return_responses", False)) if once: - return await random.choice(self.sentinels).execute_command(*args, **kwargs) + response = await random.choice(self.sentinels).execute_command( + *args, **kwargs + ) + if return_responses: + return [response] + else: + return True if response else False tasks = [ asyncio.Task(sentinel.execute_command(*args, **kwargs)) @@ -242,10 +245,10 @@ async def execute_command(self, *args, **kwargs): ] responses = await asyncio.gather(*tasks) - if bool_resp: - return reduce(lambda x, y: x and y, responses) + if return_responses: + return responses - return responses + return reduce(lambda x, y: x and y, responses) def __repr__(self): sentinel_addresses = [] diff --git a/redis/commands/sentinel.py b/redis/commands/sentinel.py index 382f9c9b12..e7f163d433 100644 --- a/redis/commands/sentinel.py +++ b/redis/commands/sentinel.py @@ -14,38 +14,55 @@ def sentinel(self, *args): def sentinel_get_master_addr_by_name(self, service_name): """Returns a (host, port) pair for the given ``service_name``""" return self.execute_command( - "SENTINEL GET-MASTER-ADDR-BY-NAME", service_name, once=True + "SENTINEL GET-MASTER-ADDR-BY-NAME", + service_name, + once=True, + return_responses=True, ) def sentinel_master(self, service_name): """Returns a dictionary containing the specified masters state.""" - return self.execute_command("SENTINEL MASTER", service_name) + return self.execute_command( + "SENTINEL MASTER", service_name, return_responses=True + ) def sentinel_masters(self): - """Returns a list of dictionaries containing each master's state.""" - return self.execute_command("SENTINEL MASTERS", once=True) + """ + Returns a list of dictionaries containing each master's state. + + Important: This function is called by the Sentinel implementation and is + called directly on the Redis standalone client for sentinels, + so it doesn'tsupport the "once" and "return_responses" options. + """ + return self.execute_command("SENTINEL MASTERS") def sentinel_monitor(self, name, ip, port, quorum): """Add a new master to Sentinel to be monitored""" - return self.execute_command( - "SENTINEL MONITOR", name, ip, port, quorum, bool_resp=True - ) + return self.execute_command("SENTINEL MONITOR", name, ip, port, quorum) def sentinel_remove(self, name): """Remove a master from Sentinel's monitoring""" - return self.execute_command("SENTINEL REMOVE", name, bool_resp=True) + return self.execute_command("SENTINEL REMOVE", name) def sentinel_sentinels(self, service_name): """Returns a list of sentinels for ``service_name``""" - return self.execute_command("SENTINEL SENTINELS", service_name) + return self.execute_command( + "SENTINEL SENTINELS", service_name, return_responses=True + ) def sentinel_set(self, name, option, value): """Set Sentinel monitoring parameters for a given master""" - return self.execute_command("SENTINEL SET", name, option, value, bool_resp=True) + return self.execute_command("SENTINEL SET", name, option, value) def sentinel_slaves(self, service_name): - """Returns a list of slaves for ``service_name``""" - return self.execute_command("SENTINEL SLAVES", service_name, once=True) + """ + Returns a list of slaves for ``service_name`` + + Important: This function is called by the Sentinel implementation and is + called directly on the Redis standalone client for sentinels, + so it doesn'tsupport the "once" and "return_responses" options. + """ + return self.execute_command("SENTINEL SLAVES", service_name) def sentinel_reset(self, pattern): """ @@ -56,9 +73,7 @@ def sentinel_reset(self, pattern): failover in progress), and removes every slave and sentinel already discovered and associated with the master. """ - return self.execute_command( - "SENTINEL RESET", pattern, once=True, bool_resp=True - ) + return self.execute_command("SENTINEL RESET", pattern, once=True) def sentinel_failover(self, new_master_name): """ @@ -67,9 +82,7 @@ def sentinel_failover(self, new_master_name): configuration will be published so that the other Sentinels will update their configurations). """ - return self.execute_command( - "SENTINEL FAILOVER", new_master_name, bool_resp=True - ) + return self.execute_command("SENTINEL FAILOVER", new_master_name) def sentinel_ckquorum(self, new_master_name): """ @@ -80,9 +93,7 @@ def sentinel_ckquorum(self, new_master_name): This command should be used in monitoring systems to check if a Sentinel deployment is ok. """ - return self.execute_command( - "SENTINEL CKQUORUM", new_master_name, once=True, bool_resp=True - ) + return self.execute_command("SENTINEL CKQUORUM", new_master_name, once=True) def sentinel_flushconfig(self): """ @@ -100,7 +111,7 @@ def sentinel_flushconfig(self): This command works even if the previous configuration file is completely missing. """ - return self.execute_command("SENTINEL FLUSHCONFIG", bool_resp=True) + return self.execute_command("SENTINEL FLUSHCONFIG") class AsyncSentinelCommands(SentinelCommands): diff --git a/redis/sentinel.py b/redis/sentinel.py index ec45ade025..74a755b1d6 100644 --- a/redis/sentinel.py +++ b/redis/sentinel.py @@ -255,26 +255,27 @@ def execute_command(self, *args, **kwargs): once - If set to True, then execute the resulting command on a single node at random, rather than across the entire sentinel cluster. """ - once = bool(kwargs.get("once", False)) - if "once" in kwargs.keys(): - kwargs.pop("once") + once = bool(kwargs.pop("once", False)) - # Check if command suppose to return boolean response. - bool_resp = bool(kwargs.get("bool_resp", False)) - if "bool_resp" in kwargs.keys(): - kwargs.pop("bool_resp") + # Check if command is supposed to return the original + # responces instead of boolean value. + return_responses = bool(kwargs.pop("return_responses", False)) if once: - return random.choice(self.sentinels).execute_command(*args, **kwargs) + response = random.choice(self.sentinels).execute_command(*args, **kwargs) + if return_responses: + return [response] + else: + return True if response else False responses = [] for sentinel in self.sentinels: responses.append(sentinel.execute_command(*args, **kwargs)) - if bool_resp: - return reduce(lambda x, y: x and y, responses) + if return_responses: + return responses - return responses + return reduce(lambda x, y: x and y, responses) def __repr__(self): sentinel_addresses = [] From 5d7fd795f3b037ea2945c7bf23ca8fa2aad421cb Mon Sep 17 00:00:00 2001 From: Petya Slavova Date: Thu, 15 May 2025 19:20:54 +0300 Subject: [PATCH 4/7] Fix several spelling mistakes --- redis/asyncio/sentinel.py | 2 +- redis/commands/sentinel.py | 4 ++-- redis/sentinel.py | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/redis/asyncio/sentinel.py b/redis/asyncio/sentinel.py index 821034bb3b..29144eb2eb 100644 --- a/redis/asyncio/sentinel.py +++ b/redis/asyncio/sentinel.py @@ -227,7 +227,7 @@ async def execute_command(self, *args, **kwargs): once = bool(kwargs.pop("once", False)) # Check if command is supposed to return the original - # responces instead of boolean value. + # responses instead of boolean value. return_responses = bool(kwargs.pop("return_responses", False)) if once: diff --git a/redis/commands/sentinel.py b/redis/commands/sentinel.py index e7f163d433..465a433e3f 100644 --- a/redis/commands/sentinel.py +++ b/redis/commands/sentinel.py @@ -32,7 +32,7 @@ def sentinel_masters(self): Important: This function is called by the Sentinel implementation and is called directly on the Redis standalone client for sentinels, - so it doesn'tsupport the "once" and "return_responses" options. + so it doesn't support the "once" and "return_responses" options. """ return self.execute_command("SENTINEL MASTERS") @@ -60,7 +60,7 @@ def sentinel_slaves(self, service_name): Important: This function is called by the Sentinel implementation and is called directly on the Redis standalone client for sentinels, - so it doesn'tsupport the "once" and "return_responses" options. + so it doesn't support the "once" and "return_responses" options. """ return self.execute_command("SENTINEL SLAVES", service_name) diff --git a/redis/sentinel.py b/redis/sentinel.py index 74a755b1d6..071379b2e2 100644 --- a/redis/sentinel.py +++ b/redis/sentinel.py @@ -258,7 +258,7 @@ def execute_command(self, *args, **kwargs): once = bool(kwargs.pop("once", False)) # Check if command is supposed to return the original - # responces instead of boolean value. + # responses instead of boolean value. return_responses = bool(kwargs.pop("return_responses", False)) if once: From 89aba81d6c77cdaa13d38d26642af9bcea0423e4 Mon Sep 17 00:00:00 2001 From: Petya Slavova Date: Fri, 16 May 2025 10:51:00 +0300 Subject: [PATCH 5/7] Adding new sentinel integration tests. Fixing the boolean return value type for sentinel's execute_command --- redis/asyncio/sentinel.py | 2 +- redis/sentinel.py | 2 +- tests/test_asyncio/test_sentinel.py | 76 ++++++++++++++++++++++++++-- tests/test_sentinel.py | 78 ++++++++++++++++++++++++++--- 4 files changed, 145 insertions(+), 13 deletions(-) diff --git a/redis/asyncio/sentinel.py b/redis/asyncio/sentinel.py index 29144eb2eb..73a20915aa 100644 --- a/redis/asyncio/sentinel.py +++ b/redis/asyncio/sentinel.py @@ -248,7 +248,7 @@ async def execute_command(self, *args, **kwargs): if return_responses: return responses - return reduce(lambda x, y: x and y, responses) + return bool(reduce(lambda x, y: x and y, responses)) def __repr__(self): sentinel_addresses = [] diff --git a/redis/sentinel.py b/redis/sentinel.py index 071379b2e2..0ab98a12b5 100644 --- a/redis/sentinel.py +++ b/redis/sentinel.py @@ -275,7 +275,7 @@ def execute_command(self, *args, **kwargs): if return_responses: return responses - return reduce(lambda x, y: x and y, responses) + return bool(reduce(lambda x, y: x and y, responses)) def __repr__(self): sentinel_addresses = [] diff --git a/tests/test_asyncio/test_sentinel.py b/tests/test_asyncio/test_sentinel.py index a27ba92bb8..1dba607b64 100644 --- a/tests/test_asyncio/test_sentinel.py +++ b/tests/test_asyncio/test_sentinel.py @@ -84,6 +84,35 @@ def sentinel(request, cluster): return Sentinel([("foo", 26379), ("bar", 26379)]) +@pytest.fixture() +async def deployed_sentinel(request): + sentinel_ips = request.config.getoption("--sentinels") + sentinel_endpoints = [ + (ip.strip(), int(port.strip())) + for ip, port in (endpoint.split(":") for endpoint in sentinel_ips.split(",")) + ] + kwargs = {} + decode_responses = True + + sentinel_kwargs = {"decode_responses": decode_responses} + force_master_ip = "localhost" + + protocol = request.config.getoption("--protocol", 2) + + sentinel = Sentinel( + sentinel_endpoints, + force_master_ip=force_master_ip, + sentinel_kwargs=sentinel_kwargs, + socket_timeout=0.1, + protocol=protocol, + decode_responses=decode_responses, + **kwargs, + ) + yield sentinel + for s in sentinel.sentinels: + await s.close() + + @pytest.mark.onlynoncluster async def test_discover_master(sentinel, master_ip): address = await sentinel.discover_master("mymaster") @@ -226,19 +255,22 @@ async def test_slave_round_robin(cluster, sentinel, master_ip): @pytest.mark.onlynoncluster -async def test_ckquorum(cluster, sentinel): - assert await sentinel.sentinel_ckquorum("mymaster") +async def test_ckquorum(sentinel): + resp = await sentinel.sentinel_ckquorum("mymaster") + assert resp is True @pytest.mark.onlynoncluster -async def test_flushconfig(cluster, sentinel): - assert await sentinel.sentinel_flushconfig() +async def test_flushconfig(sentinel): + resp = await sentinel.sentinel_flushconfig() + assert resp is True @pytest.mark.onlynoncluster async def test_reset(cluster, sentinel): cluster.master["is_odown"] = True - assert await sentinel.sentinel_reset("mymaster") + resp = await sentinel.sentinel_reset("mymaster") + assert resp is True @pytest.mark.onlynoncluster @@ -284,3 +316,37 @@ async def test_repr_correctly_represents_connection_object(sentinel): str(connection) == "" # noqa: E501 ) + + +# Tests against real sentinel instances +@pytest.mark.onlynoncluster +async def test_get_sentinels(deployed_sentinel): + resps = await deployed_sentinel.sentinel_sentinels("redis-py-test") + + # validate that the original command response is returned + assert isinstance(resps, list) + + # validate that the command has been executed against all sentinels + # each response from each sentinel is returned + assert len(resps) > 1 + + +@pytest.mark.onlynoncluster +async def test_get_master_addr_by_name(deployed_sentinel): + resps = await deployed_sentinel.sentinel_get_master_addr_by_name("redis-py-test") + + # validate that the original command response is returned + assert isinstance(resps, list) + + # validate that the command has been executed just once + # when executed once, only one response element is returned + assert len(resps) == 1 + + assert isinstance(resps[0], tuple) + + +@pytest.mark.onlynoncluster +async def test_redis_master_usage(deployed_sentinel): + r = await deployed_sentinel.master_for("redis-py-test", db=0) + await r.set("foo", "bar") + assert (await r.get("foo")) == "bar" diff --git a/tests/test_sentinel.py b/tests/test_sentinel.py index 93455f3290..ed94cd27c1 100644 --- a/tests/test_sentinel.py +++ b/tests/test_sentinel.py @@ -86,6 +86,35 @@ def sentinel(request, cluster): return Sentinel([("foo", 26379), ("bar", 26379)]) +@pytest.fixture() +def deployed_sentinel(request): + sentinel_ips = request.config.getoption("--sentinels") + sentinel_endpoints = [ + (ip.strip(), int(port.strip())) + for ip, port in (endpoint.split(":") for endpoint in sentinel_ips.split(",")) + ] + kwargs = {} + decode_responses = True + + sentinel_kwargs = {"decode_responses": decode_responses} + force_master_ip = "localhost" + + protocol = request.config.getoption("--protocol", 2) + + sentinel = Sentinel( + sentinel_endpoints, + force_master_ip=force_master_ip, + sentinel_kwargs=sentinel_kwargs, + socket_timeout=0.1, + protocol=protocol, + decode_responses=decode_responses, + **kwargs, + ) + yield sentinel + for s in sentinel.sentinels: + s.close() + + @pytest.mark.onlynoncluster def test_discover_master(sentinel, master_ip): address = sentinel.discover_master("mymaster") @@ -184,7 +213,7 @@ def test_discover_slaves(cluster, sentinel): @pytest.mark.onlynoncluster -def test_master_for(cluster, sentinel, master_ip): +def test_master_for(sentinel, master_ip): master = sentinel.master_for("mymaster", db=9) assert master.ping() assert master.connection_pool.master_address == (master_ip, 6379) @@ -228,19 +257,22 @@ def test_slave_round_robin(cluster, sentinel, master_ip): @pytest.mark.onlynoncluster -def test_ckquorum(cluster, sentinel): - assert sentinel.sentinel_ckquorum("mymaster") +def test_ckquorum(sentinel): + resp = sentinel.sentinel_ckquorum("mymaster") + assert resp is True @pytest.mark.onlynoncluster -def test_flushconfig(cluster, sentinel): - assert sentinel.sentinel_flushconfig() +def test_flushconfig(sentinel): + resp = sentinel.sentinel_flushconfig() + assert resp is True @pytest.mark.onlynoncluster def test_reset(cluster, sentinel): cluster.master["is_odown"] = True - assert sentinel.sentinel_reset("mymaster") + resp = sentinel.sentinel_reset("mymaster") + assert resp is True @pytest.mark.onlynoncluster @@ -266,3 +298,37 @@ def mock_disconnect(): assert calls == 1 pool.disconnect() + + +# Tests against real sentinel instances +@pytest.mark.onlynoncluster +def test_get_sentinels(deployed_sentinel): + resps = deployed_sentinel.sentinel_sentinels("redis-py-test") + + # validate that the original command response is returned + assert isinstance(resps, list) + + # validate that the command has been executed against all sentinels + # each response from each sentinel is returned + assert len(resps) > 1 + + +@pytest.mark.onlynoncluster +def test_get_master_addr_by_name(deployed_sentinel): + resps = deployed_sentinel.sentinel_get_master_addr_by_name("redis-py-test") + + # validate that the original command response is returned + assert isinstance(resps, list) + + # validate that the command has been executed just once + # when executed once, only one response element is returned + assert len(resps) == 1 + + assert isinstance(resps[0], tuple) + + +@pytest.mark.onlynoncluster +def test_redis_master_usage(deployed_sentinel): + r = deployed_sentinel.master_for("redis-py-test", db=0) + r.set("foo", "bar") + assert r.get("foo") == "bar" From d4ddc17babb78a87f1f4e67fcc6f39b4b0a18985 Mon Sep 17 00:00:00 2001 From: Petya Slavova Date: Thu, 5 Jun 2025 10:38:31 +0300 Subject: [PATCH 6/7] Replacing usage of reduce in the boolean return types with python's built-in 'all' function for better performance and improved readability. --- redis/asyncio/sentinel.py | 3 +-- redis/sentinel.py | 3 +-- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/redis/asyncio/sentinel.py b/redis/asyncio/sentinel.py index 73a20915aa..0bf7086555 100644 --- a/redis/asyncio/sentinel.py +++ b/redis/asyncio/sentinel.py @@ -1,7 +1,6 @@ import asyncio import random import weakref -from functools import reduce from typing import AsyncIterator, Iterable, Mapping, Optional, Sequence, Tuple, Type from redis.asyncio.client import Redis @@ -248,7 +247,7 @@ async def execute_command(self, *args, **kwargs): if return_responses: return responses - return bool(reduce(lambda x, y: x and y, responses)) + return all(responses) def __repr__(self): sentinel_addresses = [] diff --git a/redis/sentinel.py b/redis/sentinel.py index 0ab98a12b5..198639c932 100644 --- a/redis/sentinel.py +++ b/redis/sentinel.py @@ -1,6 +1,5 @@ import random import weakref -from functools import reduce from typing import Optional from redis.client import Redis @@ -275,7 +274,7 @@ def execute_command(self, *args, **kwargs): if return_responses: return responses - return bool(reduce(lambda x, y: x and y, responses)) + return all(responses) def __repr__(self): sentinel_addresses = [] From 4c88b66f56486bf9c5e416b3ae95b143048b3837 Mon Sep 17 00:00:00 2001 From: Petya Slavova Date: Thu, 5 Jun 2025 14:10:40 +0300 Subject: [PATCH 7/7] Changing the default behavior to be as before, returning of the responces is now optional. Fixed the pydocs to reflect the actual method behavior. --- redis/commands/sentinel.py | 27 ++++++++++++++++++--------- tests/test_asyncio/test_sentinel.py | 17 +++++++++++++++-- tests/test_sentinel.py | 14 ++++++++++++-- 3 files changed, 45 insertions(+), 13 deletions(-) diff --git a/redis/commands/sentinel.py b/redis/commands/sentinel.py index 465a433e3f..b2879b2015 100644 --- a/redis/commands/sentinel.py +++ b/redis/commands/sentinel.py @@ -11,19 +11,25 @@ def sentinel(self, *args): """Redis Sentinel's SENTINEL command.""" warnings.warn(DeprecationWarning("Use the individual sentinel_* methods")) - def sentinel_get_master_addr_by_name(self, service_name): - """Returns a (host, port) pair for the given ``service_name``""" + def sentinel_get_master_addr_by_name(self, service_name, return_responses=False): + """ + Returns a (host, port) pair for the given ``service_name`` when return_responses is True, + otherwise returns a boolean value that indicates if the command was successful. + """ return self.execute_command( "SENTINEL GET-MASTER-ADDR-BY-NAME", service_name, once=True, - return_responses=True, + return_responses=return_responses, ) - def sentinel_master(self, service_name): - """Returns a dictionary containing the specified masters state.""" + def sentinel_master(self, service_name, return_responses=False): + """ + Returns a dictionary containing the specified masters state, when return_responses is True, + otherwise returns a boolean value that indicates if the command was successful. + """ return self.execute_command( - "SENTINEL MASTER", service_name, return_responses=True + "SENTINEL MASTER", service_name, return_responses=return_responses ) def sentinel_masters(self): @@ -44,10 +50,13 @@ def sentinel_remove(self, name): """Remove a master from Sentinel's monitoring""" return self.execute_command("SENTINEL REMOVE", name) - def sentinel_sentinels(self, service_name): - """Returns a list of sentinels for ``service_name``""" + def sentinel_sentinels(self, service_name, return_responses=False): + """ + Returns a list of sentinels for ``service_name``, when return_responses is True, + otherwise returns a boolean value that indicates if the command was successful. + """ return self.execute_command( - "SENTINEL SENTINELS", service_name, return_responses=True + "SENTINEL SENTINELS", service_name, return_responses=return_responses ) def sentinel_set(self, name, option, value): diff --git a/tests/test_asyncio/test_sentinel.py b/tests/test_asyncio/test_sentinel.py index 1dba607b64..8d8b8f512c 100644 --- a/tests/test_asyncio/test_sentinel.py +++ b/tests/test_asyncio/test_sentinel.py @@ -321,7 +321,9 @@ async def test_repr_correctly_represents_connection_object(sentinel): # Tests against real sentinel instances @pytest.mark.onlynoncluster async def test_get_sentinels(deployed_sentinel): - resps = await deployed_sentinel.sentinel_sentinels("redis-py-test") + resps = await deployed_sentinel.sentinel_sentinels( + "redis-py-test", return_responses=True + ) # validate that the original command response is returned assert isinstance(resps, list) @@ -330,10 +332,17 @@ async def test_get_sentinels(deployed_sentinel): # each response from each sentinel is returned assert len(resps) > 1 + # validate default behavior + resps = await deployed_sentinel.sentinel_sentinels("redis-py-test") + assert isinstance(resps, bool) + @pytest.mark.onlynoncluster async def test_get_master_addr_by_name(deployed_sentinel): - resps = await deployed_sentinel.sentinel_get_master_addr_by_name("redis-py-test") + resps = await deployed_sentinel.sentinel_get_master_addr_by_name( + "redis-py-test", + return_responses=True, + ) # validate that the original command response is returned assert isinstance(resps, list) @@ -344,6 +353,10 @@ async def test_get_master_addr_by_name(deployed_sentinel): assert isinstance(resps[0], tuple) + # validate default behavior + resps = await deployed_sentinel.sentinel_get_master_addr_by_name("redis-py-test") + assert isinstance(resps, bool) + @pytest.mark.onlynoncluster async def test_redis_master_usage(deployed_sentinel): diff --git a/tests/test_sentinel.py b/tests/test_sentinel.py index ed94cd27c1..0e7624c836 100644 --- a/tests/test_sentinel.py +++ b/tests/test_sentinel.py @@ -303,7 +303,7 @@ def mock_disconnect(): # Tests against real sentinel instances @pytest.mark.onlynoncluster def test_get_sentinels(deployed_sentinel): - resps = deployed_sentinel.sentinel_sentinels("redis-py-test") + resps = deployed_sentinel.sentinel_sentinels("redis-py-test", return_responses=True) # validate that the original command response is returned assert isinstance(resps, list) @@ -312,10 +312,16 @@ def test_get_sentinels(deployed_sentinel): # each response from each sentinel is returned assert len(resps) > 1 + # validate default behavior + resps = deployed_sentinel.sentinel_sentinels("redis-py-test") + assert isinstance(resps, bool) + @pytest.mark.onlynoncluster def test_get_master_addr_by_name(deployed_sentinel): - resps = deployed_sentinel.sentinel_get_master_addr_by_name("redis-py-test") + resps = deployed_sentinel.sentinel_get_master_addr_by_name( + "redis-py-test", return_responses=True + ) # validate that the original command response is returned assert isinstance(resps, list) @@ -326,6 +332,10 @@ def test_get_master_addr_by_name(deployed_sentinel): assert isinstance(resps[0], tuple) + # validate default behavior + resps = deployed_sentinel.sentinel_get_master_addr_by_name("redis-py-test") + assert isinstance(resps, bool) + @pytest.mark.onlynoncluster def test_redis_master_usage(deployed_sentinel):