From e281f4f1936094be24f81d1fafbf8d17caa1e728 Mon Sep 17 00:00:00 2001 From: Sergey Prokazov Date: Tue, 7 Feb 2023 17:31:37 -0600 Subject: [PATCH 1/3] Fix #2581 UnixDomainSocketConnection' object has no attribute '_command_packer' . Apparently there is no end-to-end tests for Unix sockets so automation didn't catch it. I assume that setting up domain sockets reliably in dockerized environment is not very trivial. Added test for pack_command specifically. --- redis/connection.py | 2 ++ tests/test_connection.py | 46 +++++++++++++++++++++++++++++++++++++++- 2 files changed, 47 insertions(+), 1 deletion(-) diff --git a/redis/connection.py b/redis/connection.py index 24614824c5..d35980c167 100644 --- a/redis/connection.py +++ b/redis/connection.py @@ -1153,6 +1153,7 @@ def __init__( retry=None, redis_connect_func=None, credential_provider: Optional[CredentialProvider] = None, + command_packer=None, ): """ Initialize a new UnixDomainSocketConnection. @@ -1202,6 +1203,7 @@ def __init__( self.set_parser(parser_class) self._connect_callbacks = [] self._buffer_cutoff = 6000 + self._command_packer = self._construct_command_packer(command_packer) def repr_pieces(self): pieces = [("path", self.path), ("db", self.db)] diff --git a/tests/test_connection.py b/tests/test_connection.py index e0b53cdf37..e7879d306a 100644 --- a/tests/test_connection.py +++ b/tests/test_connection.py @@ -7,7 +7,13 @@ import redis from redis.backoff import NoBackoff -from redis.connection import Connection, HiredisParser, PythonParser +from redis.connection import ( + Connection, + HiredisParser, + PythonParser, + SSLConnection, + UnixDomainSocketConnection, +) from redis.exceptions import ConnectionError, InvalidResponse, TimeoutError from redis.retry import Retry from redis.utils import HIREDIS_AVAILABLE @@ -163,3 +169,41 @@ def test_connection_parse_response_resume(r: redis.Redis, parser_class): pytest.fail("didn't receive a response") assert response assert i > 0 + + +@pytest.mark.onlynoncluster +@pytest.mark.parametrize( + "Class", + [ + Connection, + SSLConnection, + UnixDomainSocketConnection, + ], +) +def test_pack_command(Class): + """ + This test verifies that the pack_command works + on all supported connections. #2581 + """ + cmd = ( + "HSET", + "foo", + "key", + "value1", + b"key_b", + b"bytes str", + "key_mv", + memoryview(b"bytes str"), + b"key_i", + 67, + "key_f", + 3.14159265359, + ) + expected = ( + b"*12\r\n$4\r\nHSET\r\n$3\r\nfoo\r\n$3\r\nkey\r\n$6\r\nvalue1\r\n" + b"$5\r\nkey_b\r\n$9\r\nbytes str\r\n$6\r\nkey_mv\r\n$9\r\n" + b"bytes str\r\n$5\r\nkey_i\r\n$2\r\n67\r\n$5\r\nkey_f\r\n$13\r\n" + b"3.14159265359\r\n" + ) + + assert Class().pack_command(*cmd)[0] == expected From 8ff7d501241a6f657df84214041215505006d2c1 Mon Sep 17 00:00:00 2001 From: Sergey Prokazov Date: Tue, 7 Feb 2023 18:11:40 -0600 Subject: [PATCH 2/3] Figuring out why CI fails. Locally: " congratulations :)" --- tests/test_connection.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/test_connection.py b/tests/test_connection.py index e7879d306a..0f265bcf2e 100644 --- a/tests/test_connection.py +++ b/tests/test_connection.py @@ -206,4 +206,5 @@ def test_pack_command(Class): b"3.14159265359\r\n" ) - assert Class().pack_command(*cmd)[0] == expected + actual = Class().pack_command(*cmd)[0] + assert actual == expected, f"actual = {actual}, expected = {expected}" From b6664d380d515b675f9c1e8fb5a2a79cc50d30da Mon Sep 17 00:00:00 2001 From: Sergey Prokazov Date: Tue, 7 Feb 2023 19:23:43 -0600 Subject: [PATCH 3/3] Fix the test. hiredis doesn't treat memoryviews differently. --- tests/test_connection.py | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/tests/test_connection.py b/tests/test_connection.py index 0f265bcf2e..25b4118b2c 100644 --- a/tests/test_connection.py +++ b/tests/test_connection.py @@ -192,18 +192,15 @@ def test_pack_command(Class): "value1", b"key_b", b"bytes str", - "key_mv", - memoryview(b"bytes str"), b"key_i", 67, "key_f", 3.14159265359, ) expected = ( - b"*12\r\n$4\r\nHSET\r\n$3\r\nfoo\r\n$3\r\nkey\r\n$6\r\nvalue1\r\n" - b"$5\r\nkey_b\r\n$9\r\nbytes str\r\n$6\r\nkey_mv\r\n$9\r\n" - b"bytes str\r\n$5\r\nkey_i\r\n$2\r\n67\r\n$5\r\nkey_f\r\n$13\r\n" - b"3.14159265359\r\n" + b"*10\r\n$4\r\nHSET\r\n$3\r\nfoo\r\n$3\r\nkey\r\n$6\r\nvalue1\r\n" + b"$5\r\nkey_b\r\n$9\r\nbytes str\r\n$5\r\nkey_i\r\n$2\r\n67\r\n$5" + b"\r\nkey_f\r\n$13\r\n3.14159265359\r\n" ) actual = Class().pack_command(*cmd)[0]