From d3f4020717d8a2c834bb1b4035a0dbcd93b2c814 Mon Sep 17 00:00:00 2001 From: Shay Fadida Date: Sun, 24 Jul 2022 18:15:42 +0300 Subject: [PATCH 1/3] Fix special response parsing options handling When using special response parsing options like `NEVER_DECODE` and `EMPTY_RESPONSE`, don't pass them to the response callbacks because some of them are not prepared for receiving named arguments. Instead, redis-py should use them before calling the callbacks and then discard them. --- redis/asyncio/client.py | 5 +++++ redis/asyncio/cluster.py | 4 ++++ redis/client.py | 5 +++++ tests/test_asyncio/test_commands.py | 12 +++++++++++- tests/test_commands.py | 12 +++++++++++- 5 files changed, 36 insertions(+), 2 deletions(-) diff --git a/redis/asyncio/client.py b/redis/asyncio/client.py index 0e40ed70f8..6c0b815fc9 100644 --- a/redis/asyncio/client.py +++ b/redis/asyncio/client.py @@ -501,12 +501,17 @@ async def parse_response( try: if NEVER_DECODE in options: response = await connection.read_response(disable_decoding=True) + options.pop(NEVER_DECODE) else: response = await connection.read_response() except ResponseError: if EMPTY_RESPONSE in options: return options[EMPTY_RESPONSE] raise + + if EMPTY_RESPONSE in options: + options.pop(EMPTY_RESPONSE) + if command_name in self.response_callbacks: # Mypy bug: https://github.com/python/mypy/issues/10977 command_name = cast(str, command_name) diff --git a/redis/asyncio/cluster.py b/redis/asyncio/cluster.py index 8d34b9ad21..af5fbb2bd7 100644 --- a/redis/asyncio/cluster.py +++ b/redis/asyncio/cluster.py @@ -867,6 +867,7 @@ async def parse_response( try: if NEVER_DECODE in kwargs: response = await connection.read_response(disable_decoding=True) + options.pop(NEVER_DECODE) else: response = await connection.read_response() except ResponseError: @@ -874,6 +875,9 @@ async def parse_response( return kwargs[EMPTY_RESPONSE] raise + if EMPTY_RESPONSE in options: + options.pop(EMPTY_RESPONSE) + # Return response if command in self.response_callbacks: return self.response_callbacks[command](response, **kwargs) diff --git a/redis/client.py b/redis/client.py index 75a0dac226..5edd66da11 100755 --- a/redis/client.py +++ b/redis/client.py @@ -1258,12 +1258,17 @@ def parse_response(self, connection, command_name, **options): try: if NEVER_DECODE in options: response = connection.read_response(disable_decoding=True) + options.pop(NEVER_DECODE) else: response = connection.read_response() except ResponseError: if EMPTY_RESPONSE in options: return options[EMPTY_RESPONSE] raise + + if EMPTY_RESPONSE in options: + options.pop(EMPTY_RESPONSE) + if command_name in self.response_callbacks: return self.response_callbacks[command_name](response, **options) return response diff --git a/tests/test_asyncio/test_commands.py b/tests/test_asyncio/test_commands.py index 1242e04679..67471bb4b3 100644 --- a/tests/test_asyncio/test_commands.py +++ b/tests/test_asyncio/test_commands.py @@ -11,7 +11,7 @@ import redis from redis import exceptions -from redis.client import parse_info +from redis.client import EMPTY_RESPONSE, NEVER_DECODE, parse_info from tests.conftest import ( skip_if_server_version_gte, skip_if_server_version_lt, @@ -542,6 +542,16 @@ async def test_time(self, r: redis.Redis): assert isinstance(t[0], int) assert isinstance(t[1], int) + async def test_never_decode_option(self, r: redis.Redis): + opts = {NEVER_DECODE: []} + await r.delete("a") + assert await r.execute_command("EXISTS", "a", **opts) == 0 + + async def test_empty_response_option(self, r: redis.Redis): + opts = {EMPTY_RESPONSE: []} + await r.delete("a") + assert await r.execute_command("EXISTS", "a", **opts) == 0 + # BASIC KEY COMMANDS async def test_append(self, r: redis.Redis): assert await r.append("a", "a1") == 2 diff --git a/tests/test_commands.py b/tests/test_commands.py index 1c9a5c27eb..aa6745b34b 100644 --- a/tests/test_commands.py +++ b/tests/test_commands.py @@ -9,7 +9,7 @@ import redis from redis import exceptions -from redis.client import parse_info +from redis.client import EMPTY_RESPONSE, NEVER_DECODE, parse_info from .conftest import ( _get_client, @@ -917,6 +917,16 @@ def test_bgsave(self, r): time.sleep(0.3) assert r.bgsave(True) + def test_never_decode_option(self, r: redis.Redis): + opts = {NEVER_DECODE: []} + r.delete("a") + assert r.execute_command("EXISTS", "a", **opts) == 0 + + def test_empty_response_option(self, r: redis.Redis): + opts = {EMPTY_RESPONSE: []} + r.delete("a") + assert r.execute_command("EXISTS", "a", **opts) == 0 + # BASIC KEY COMMANDS def test_append(self, r): assert r.append("a", "a1") == 2 From 27efde36562a50b3d55af44a0bbbccf3dbc787f8 Mon Sep 17 00:00:00 2001 From: Shay Fadida Date: Wed, 9 Nov 2022 10:10:25 +0200 Subject: [PATCH 2/3] Use kwargs instead of options --- redis/asyncio/cluster.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/redis/asyncio/cluster.py b/redis/asyncio/cluster.py index af5fbb2bd7..cb035e12f7 100644 --- a/redis/asyncio/cluster.py +++ b/redis/asyncio/cluster.py @@ -867,7 +867,7 @@ async def parse_response( try: if NEVER_DECODE in kwargs: response = await connection.read_response(disable_decoding=True) - options.pop(NEVER_DECODE) + kwargs.pop(NEVER_DECODE) else: response = await connection.read_response() except ResponseError: @@ -876,7 +876,7 @@ async def parse_response( raise if EMPTY_RESPONSE in options: - options.pop(EMPTY_RESPONSE) + kwargs.pop(EMPTY_RESPONSE) # Return response if command in self.response_callbacks: From c3ee2c99d9cb168065ec021bf55487c64d87dd83 Mon Sep 17 00:00:00 2001 From: dvora-h <67596500+dvora-h@users.noreply.github.com> Date: Wed, 9 Nov 2022 10:58:58 +0200 Subject: [PATCH 3/3] change options to kwargs in asyncio/cluster.py/L878 --- redis/asyncio/cluster.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/redis/asyncio/cluster.py b/redis/asyncio/cluster.py index cb035e12f7..924c604b6f 100644 --- a/redis/asyncio/cluster.py +++ b/redis/asyncio/cluster.py @@ -875,7 +875,7 @@ async def parse_response( return kwargs[EMPTY_RESPONSE] raise - if EMPTY_RESPONSE in options: + if EMPTY_RESPONSE in kwargs: kwargs.pop(EMPTY_RESPONSE) # Return response