Fix bug with partial Hiredis availability (#3400)

* Fix bug with partial Hiredis availability * Added yes flag * Codestyl fixes * Removed redundant check * Removed redundant checks associated with pack command * Updated condition to check the actual flag * Removed unused import
redis · Oct 4, 2024 · aa22225 · aa22225
1 parent c31849c
commit aa22225
Show file tree

Hide file tree

Showing 4 changed files with 36 additions and 22 deletions.
diff --git a/.github/workflows/integration.yaml b/.github/workflows/integration.yaml
@@ -90,6 +90,21 @@ jobs:
           invoke ${{matrix.test-type}}-tests
           ls -1
 
+      - name: Run tests against hiredis < 3.0.0
+        if: ${{ matrix.connection-type == 'hiredis' && matrix.python-version == '3.12'}}
+        run: |
+          pip uninstall hiredis -y
+          pip install -U setuptools wheel
+          pip install -r requirements.txt
+          pip install -r dev_requirements.txt
+          if [ "${{matrix.connection-type}}" == "hiredis" ]; then
+           pip install "hiredis<3.0.0"
+          fi
+          invoke devenv
+          sleep 10 # time to settle
+          invoke ${{matrix.test-type}}-tests
+          ls -1
+
       - name: Upload test results and profiling data
         uses: actions/upload-artifact@v4
         with:
@@ -145,6 +160,24 @@ jobs:
             invoke ${{matrix.test-type}}-tests --protocol=3
           fi
 
+      - name: Run tests against hiredis < 3.0.0
+        if: ${{ matrix.connection-type == 'hiredis' && matrix.python-version == '3.12'}}
+        run: |
+          pip uninstall hiredis -y
+          pip install -U setuptools wheel
+          pip install -r requirements.txt
+          pip install -r dev_requirements.txt
+          if [ "${{matrix.connection-type}}" == "hiredis" ]; then
+            pip install "hiredis<3.0.0"
+          fi
+          invoke devenv
+          sleep 10 # time to settle
+          if [ "${{matrix.event-loop}}" == "uvloop" ]; then
+            invoke ${{matrix.test-type}}-tests --uvloop --protocol=3
+          else
+            invoke ${{matrix.test-type}}-tests --protocol=3
+          fi
+
       - name: Upload test results and profiling data
         uses: actions/upload-artifact@v4
         with:

diff --git a/redis/connection.py b/redis/connection.py
@@ -38,7 +38,6 @@
 from .utils import (
     CRYPTOGRAPHY_AVAILABLE,
     HIREDIS_AVAILABLE,
-    HIREDIS_PACK_AVAILABLE,
     SSL_AVAILABLE,
     compare_versions,
     ensure_string,
@@ -314,7 +313,7 @@ def __del__(self):
     def _construct_command_packer(self, packer):
         if packer is not None:
             return packer
-        elif HIREDIS_PACK_AVAILABLE:
+        elif HIREDIS_AVAILABLE:
             return HiredisRespSerializer()
         else:
             return PythonRespSerializer(self._buffer_cutoff, self.encoder.encode)

diff --git a/redis/utils.py b/redis/utils.py
@@ -8,10 +8,10 @@
 
     # Only support Hiredis >= 3.0:
     HIREDIS_AVAILABLE = int(hiredis.__version__.split(".")[0]) >= 3
-    HIREDIS_PACK_AVAILABLE = hasattr(hiredis, "pack_command")
+    if not HIREDIS_AVAILABLE:
+        raise ImportError("hiredis package should be >= 3.0.0")
 except ImportError:
     HIREDIS_AVAILABLE = False
-    HIREDIS_PACK_AVAILABLE = False
 
 try:
     import ssl  # noqa

diff --git a/tests/test_encoding.py b/tests/test_encoding.py
@@ -1,7 +1,5 @@
 import pytest
 import redis
-from redis.connection import Connection
-from redis.utils import HIREDIS_PACK_AVAILABLE
 
 from .conftest import _get_client
 
@@ -75,22 +73,6 @@ def test_replace(self, request):
         assert r.get("a") == "foo\ufffd"
 
 
-@pytest.mark.skipif(
-    HIREDIS_PACK_AVAILABLE,
-    reason="Packing via hiredis does not preserve memoryviews",
-)
-class TestMemoryviewsAreNotPacked:
-    def test_memoryviews_are_not_packed(self):
-        c = Connection()
-        arg = memoryview(b"some_arg")
-        arg_list = ["SOME_COMMAND", arg]
-        cmd = c.pack_command(*arg_list)
-        assert cmd[1] is arg
-        cmds = c.pack_commands([arg_list, arg_list])
-        assert cmds[1] is arg
-        assert cmds[3] is arg
-
-
 class TestCommandsAreNotEncoded:
     @pytest.fixture()
     def r(self, request):