Add support for Python 3.12 (redis#2979)

Add support for Python 3.12. This required a bunch of changes all
over the place, listed below in a random order.

Fix tests, especially around SSL connections.

Stop requiring typing-extensions.

Enable tracemalloc for tests, and add the possibility to run the tests
with profiling enabled. Fix some issues identified by tracemalloc, like
sockets not being closed in case of SSL handshake failures.

Remove the CI test reporting plugin, it does not work easily with forked
repos anyway.

Fix checking of module versions, make the comparison accurate. Not
sure how it worked before, but it looks like it did not match exactly
the format in the server INFO response, i.e. MMmmPP.

Remove loggers from tests, it's just noise in the output. If we don't use
asserts, nobody will check the log output from CI.

Speed up the computation for slots when initializing a cluster. After
profiling, this turned out to be very slow, when it does not have to be.
It does not make sense to recompute the same thing over and over
in a loop.

Run uvloop tests in matrix, i.e. don't bundle two tests executions
(without uvloop and with it) in the same job. Easier to spot failures
like this, and arguably the jobs can be scheduled in parallel so the 
overall execution is faster.

Unlock urllib version, to be able to use more recent pytest versions.

---------

Co-authored-by: Gabriel Erzse <[email protected]>

Author: chayim

.flake8

@@ -16,6 +16,7 @@ exclude =
 ignore =
     E126
     E203
+    E231
     E701
     E704
     F405

.github/workflows/integration.yaml

@@ -25,6 +25,8 @@ permissions:
 
 env:
   CODECOV_TOKEN: ${{ secrets.CODECOV_TOKEN }}
+  # this speeds up coverage with Python 3.12: https://github.com/nedbat/coveragepy/issues/1665
+  COVERAGE_CORE: sysmon
   REDIS_IMAGE: redis:7.4-rc2
   REDIS_STACK_IMAGE: redis/redis-stack-server:7.4.0-rc2
 
@@ -54,26 +56,28 @@ jobs:
           pip install -r dev_requirements.txt
           invoke linters
 
-  run-tests:
+  resp2-tests:
     runs-on: ubuntu-latest
     timeout-minutes: 60
     strategy:
       max-parallel: 15
       fail-fast: false
       matrix:
-        python-version: ['3.8', '3.9', '3.10', '3.11', 'pypy-3.8', 'pypy-3.9']
+        python-version: ['3.8', '3.9', '3.10', '3.11', '3.12', 'pypy-3.8', 'pypy-3.9']
         test-type: ['standalone', 'cluster']
         connection-type: ['hiredis', 'plain']
     env:
       ACTIONS_ALLOW_UNSECURE_COMMANDS: true
-    name: Python ${{ matrix.python-version }} ${{matrix.test-type}}-${{matrix.connection-type}} tests
+    name: RESP2 ${{ matrix.python-version }} ${{matrix.test-type}}-${{matrix.connection-type}}
     steps:
       - uses: actions/checkout@v4
+
       - uses: actions/setup-python@v5
         with:
           python-version: ${{ matrix.python-version }}
           cache: 'pip'
-      - name: run tests
+
+      - name: Run tests
         run: |
           pip install -U setuptools wheel
           pip install -r requirements.txt
@@ -84,52 +88,48 @@ jobs:
           invoke devenv
           sleep 10 # time to settle
           invoke ${{matrix.test-type}}-tests
+          ls -1
 
-      - uses: actions/upload-artifact@v4
-        if: success() || failure()
+      - name: Upload test results and profiling data
+        uses: actions/upload-artifact@v4
         with:
           name: pytest-results-${{matrix.test-type}}-${{matrix.connection-type}}-${{matrix.python-version}}
-          path: '${{matrix.test-type}}*results.xml'
+          path: |
+            ${{matrix.test-type}}*-results.xml
+            prof/**
+            profile_output*
+          if-no-files-found: error
+          retention-days: 10
 
       - name: Upload codecov coverage
         uses: codecov/codecov-action@v4
         with:
           fail_ci_if_error: false
 
-      - name: View Test Results
-        uses: dorny/test-reporter@v1
-        if: success() || failure()
-        continue-on-error: true
-        with:
-          name: Test Results ${{matrix.python-version}} ${{matrix.test-type}}-${{matrix.connection-type}}
-          path: '*.xml'
-          reporter: java-junit
-          list-suites: all
-          list-tests: all
-          max-annotations: 10
-          fail-on-error: 'false'
-
-  resp3_tests:
+  resp3-tests:
     runs-on: ubuntu-latest
     strategy:
       fail-fast: false
       matrix:
-        python-version: ['3.8', '3.11']
+        python-version: ['3.8', '3.12']
         test-type: ['standalone', 'cluster']
         connection-type: ['hiredis', 'plain']
+        event-loop: ['asyncio', 'uvloop']
         exclude:
           - test-type: 'cluster'
             connection-type: 'hiredis'
     env:
       ACTIONS_ALLOW_UNSECURE_COMMANDS: true
-    name: RESP3 [${{ matrix.python-version }} ${{matrix.test-type}}-${{matrix.connection-type}}]
+    name: RESP3 ${{ matrix.python-version }} ${{matrix.test-type}}-${{matrix.connection-type}}-${{matrix.event-loop}}
     steps:
       - uses: actions/checkout@v4
+
       - uses: actions/setup-python@v5
         with:
           python-version: ${{ matrix.python-version }}
           cache: 'pip'
-      - name: run tests
+
+      - name: Run tests
         run: |
           pip install -U setuptools wheel
           pip install -r requirements.txt
@@ -139,37 +139,32 @@ jobs:
           fi
           invoke devenv
           sleep 10 # time to settle
-          invoke ${{matrix.test-type}}-tests --protocol=3
-          invoke ${{matrix.test-type}}-tests --uvloop --protocol=3
+          if [ "${{matrix.event-loop}}" == "uvloop" ]; then
+            invoke ${{matrix.test-type}}-tests --uvloop --protocol=3
+          else
+            invoke ${{matrix.test-type}}-tests --protocol=3
+          fi
 
-      - uses: actions/upload-artifact@v4
-        if: success() || failure()
+      - name: Upload test results and profiling data
+        uses: actions/upload-artifact@v4
         with:
-          name: pytest-results-${{matrix.test-type}}-${{matrix.connection-type}}-${{matrix.python-version}}-resp3
-          path: '${{matrix.test-type}}*results.xml'
+          name: pytest-results-${{matrix.test-type}}-${{matrix.connection-type}}-${{matrix.python-version}}-${{matrix.event-loop}}-resp3
+          path: |
+            ${{matrix.test-type}}*-results.xml
+            prof/**
+            profile_output*
+          if-no-files-found: error
+          retention-days: 10
 
       - name: Upload codecov coverage
         uses: codecov/codecov-action@v4
         with:
           fail_ci_if_error: false
 
-      - name: View Test Results
-        uses: dorny/test-reporter@v1
-        if: success() || failure()
-        continue-on-error: true
-        with:
-          name: Test Results ${{matrix.python-version}} ${{matrix.test-type}}-${{matrix.connection-type}}-resp3
-          path: '*.xml'
-          reporter: java-junit
-          list-suites: all
-          list-tests: all
-          max-annotations: 10
-          fail-on-error: 'false'
-
-  build_and_test_package:
+  build-and-test-package:
     name: Validate building and installing the package
     runs-on: ubuntu-latest
-    needs: [run-tests]
+    needs: [resp2-tests, resp3-tests]
     strategy:
       fail-fast: false
       matrix:
@@ -183,13 +178,13 @@ jobs:
         run: |
           bash .github/workflows/install_and_test.sh ${{ matrix.extension }}
 
-  install_package_from_commit:
+  install-package-from-commit:
     name: Install package from commit hash
     runs-on: ubuntu-latest
     strategy:
       fail-fast: false
       matrix:
-        python-version: ['3.8', '3.9', '3.10', '3.11', 'pypy-3.8', 'pypy-3.9']
+        python-version: ['3.8', '3.9', '3.10', '3.11', '3.12', 'pypy-3.8', 'pypy-3.9']
     steps:
       - uses: actions/checkout@v4
       - uses: actions/setup-python@v5

.gitignore

@@ -16,4 +16,6 @@ coverage.xml
 .venv*
 *.xml
 .coverage*
+prof
+profile_output*
 docker/stunnel/keys

dev_requirements.txt

@@ -4,15 +4,15 @@ click==8.0.4
 flake8-isort==6.0.0
 flake8==5.0.4
 flynt~=0.69.0
-invoke==1.7.3
-mock==4.0.3
+invoke==2.2.0
+mock
 packaging>=20.4
 pytest
 pytest-asyncio
 pytest-cov
+pytest-profiling
 pytest-timeout
 ujson>=4.2.0
-urllib3<2
 uvloop
 vulture>=2.3.0
 wheel>=0.30.0

redis/asyncio/cluster.py

@@ -1317,37 +1317,35 @@ async def initialize(self) -> None:
                 port = int(primary_node[1])
                 host, port = self.remap_host_port(host, port)
 
+                nodes_for_slot = []
+
                 target_node = tmp_nodes_cache.get(get_node_name(host, port))
                 if not target_node:
                     target_node = ClusterNode(
                         host, port, PRIMARY, **self.connection_kwargs
                     )
                 # add this node to the nodes cache
                 tmp_nodes_cache[target_node.name] = target_node
+                nodes_for_slot.append(target_node)
+
+                replica_nodes = slot[3:]
+                for replica_node in replica_nodes:
+                    host = replica_node[0]
+                    port = replica_node[1]
+                    host, port = self.remap_host_port(host, port)
+
+                    target_replica_node = tmp_nodes_cache.get(get_node_name(host, port))
+                    if not target_replica_node:
+                        target_replica_node = ClusterNode(
+                            host, port, REPLICA, **self.connection_kwargs
+                        )
+                    # add this node to the nodes cache
+                    tmp_nodes_cache[target_replica_node.name] = target_replica_node
+                    nodes_for_slot.append(target_replica_node)
 
                 for i in range(int(slot[0]), int(slot[1]) + 1):
                     if i not in tmp_slots:
-                        tmp_slots[i] = []
-                        tmp_slots[i].append(target_node)
-                        replica_nodes = [slot[j] for j in range(3, len(slot))]
-
-                        for replica_node in replica_nodes:
-                            host = replica_node[0]
-                            port = replica_node[1]
-                            host, port = self.remap_host_port(host, port)
-
-                            target_replica_node = tmp_nodes_cache.get(
-                                get_node_name(host, port)
-                            )
-                            if not target_replica_node:
-                                target_replica_node = ClusterNode(
-                                    host, port, REPLICA, **self.connection_kwargs
-                                )
-                            tmp_slots[i].append(target_replica_node)
-                            # add this node to the nodes cache
-                            tmp_nodes_cache[target_replica_node.name] = (
-                                target_replica_node
-                            )
+                        tmp_slots[i] = nodes_for_slot
                     else:
                         # Validate that 2 nodes want to use the same slot cache
                         # setup

redis/asyncio/connection.py

@@ -1000,7 +1000,7 @@ def parse_url(url: str) -> ConnectKwargs:
                 try:
                     kwargs[name] = parser(value)
                 except (TypeError, ValueError):
-                    raise ValueError(f"Invalid value for `{name}` in connection URL.")
+                    raise ValueError(f"Invalid value for '{name}' in connection URL.")
             else:
                 kwargs[name] = value
 

redis/cluster.py

@@ -1522,6 +1522,8 @@ def _get_or_create_cluster_node(self, host, port, role, tmp_nodes_cache):
                 target_node = ClusterNode(host, port, role)
             if target_node.server_type != role:
                 target_node.server_type = role
+            # add this node to the nodes cache
+            tmp_nodes_cache[target_node.name] = target_node
 
         return target_node
 
@@ -1585,31 +1587,26 @@ def initialize(self):
                 port = int(primary_node[1])
                 host, port = self.remap_host_port(host, port)
 
+                nodes_for_slot = []
+
                 target_node = self._get_or_create_cluster_node(
                     host, port, PRIMARY, tmp_nodes_cache
                 )
-                # add this node to the nodes cache
-                tmp_nodes_cache[target_node.name] = target_node
+                nodes_for_slot.append(target_node)
+
+                replica_nodes = slot[3:]
+                for replica_node in replica_nodes:
+                    host = str_if_bytes(replica_node[0])
+                    port = int(replica_node[1])
+                    host, port = self.remap_host_port(host, port)
+                    target_replica_node = self._get_or_create_cluster_node(
+                        host, port, REPLICA, tmp_nodes_cache
+                    )
+                    nodes_for_slot.append(target_replica_node)
 
                 for i in range(int(slot[0]), int(slot[1]) + 1):
                     if i not in tmp_slots:
-                        tmp_slots[i] = []
-                        tmp_slots[i].append(target_node)
-                        replica_nodes = [slot[j] for j in range(3, len(slot))]
-
-                        for replica_node in replica_nodes:
-                            host = str_if_bytes(replica_node[0])
-                            port = replica_node[1]
-                            host, port = self.remap_host_port(host, port)
-
-                            target_replica_node = self._get_or_create_cluster_node(
-                                host, port, REPLICA, tmp_nodes_cache
-                            )
-                            tmp_slots[i].append(target_replica_node)
-                            # add this node to the nodes cache
-                            tmp_nodes_cache[target_replica_node.name] = (
-                                target_replica_node
-                            )
+                        tmp_slots[i] = nodes_for_slot
                     else:
                         # Validate that 2 nodes want to use the same slot cache
                         # setup

redis/commands/graph/query_result.py

@@ -1,6 +1,5 @@
 import sys
 from collections import OrderedDict
-from distutils.util import strtobool
 
 # from prettytable import PrettyTable
 from redis import ResponseError
@@ -571,3 +570,19 @@ async def parse_array(self, value):
         """
         scalar = [await self.parse_scalar(value[i]) for i in range(len(value))]
         return scalar
+
+
+def strtobool(val):
+    """
+    Convert a string representation of truth to true (1) or false (0).
+    True values are 'y', 'yes', 't', 'true', 'on', and '1'; false values
+    are 'n', 'no', 'f', 'false', 'off', and '0'.  Raises ValueError if
+    'val' is anything else.
+    """
+    val = val.lower()
+    if val in ("y", "yes", "t", "true", "on", "1"):
+        return True
+    elif val in ("n", "no", "f", "false", "off", "0"):
+        return False
+    else:
+        raise ValueError(f"invalid truth value {val!r}")

redis/connection.py

@@ -813,8 +813,26 @@ def __init__(
         super().__init__(**kwargs)
 
     def _connect(self):
-        "Wrap the socket with SSL support"
+        """
+        Wrap the socket with SSL support, handling potential errors.
+        """
         sock = super()._connect()
+        try:
+            return self._wrap_socket_with_ssl(sock)
+        except OSError:
+            sock.close()
+            raise
+
+    def _wrap_socket_with_ssl(self, sock):
+        """
+        Wraps the socket with SSL support.
+
+        Args:
+            sock: The plain socket to wrap with SSL.
+
+        Returns:
+            An SSL wrapped socket.
+        """
         context = ssl.create_default_context()
         context.check_hostname = self.check_hostname
         context.verify_mode = self.cert_reqs
@@ -957,7 +975,7 @@ def parse_url(url):
                 try:
                     kwargs[name] = parser(value)
                 except (TypeError, ValueError):
-                    raise ValueError(f"Invalid value for `{name}` in connection URL.")
+                    raise ValueError(f"Invalid value for '{name}' in connection URL.")
             else:
                 kwargs[name] = value