New redis fixes (#182)

* Remove jobs running with the eol manylinux2010 os * Fix issues with the newer version of the redis bindings * Fix issues with the newer version of the redis bindings * Remove redis.submodule that shouldn't be in the git repo * Remove redis.submodule that shouldn't be in the git repo * Code coverage tweaks --------- Co-authored-by: Dwight Hubbard <[email protected]>
yahoo · Sep 18, 2023 · 95fcad1 · 95fcad1
1 parent e2377bd
commit 95fcad1
Show file tree

Hide file tree

Showing 11 changed files with 109 additions and 30 deletions.
diff --git a/.gitignore b/.gitignore
@@ -84,10 +84,16 @@ __pycache__/
 .idea/
 redislite/package_metadata.json
 
+# Pycharm venv
+/venv
+
 # testing artifacts
 cobertura.xml
 test_unit_redis.db
 
+# Redis submoudle should only be updated in ci/cd
+/redis.submodule/
+
 # redis build artifacts
 redis.submodule/**/*.o
 redis.submodule/**/.make-*

diff --git a/MANIFEST.in b/MANIFEST.in
@@ -3,7 +3,6 @@
 # See the accompanying LICENSE.txt file for terms.
 
 # metadata
-include README.rst
 include README.md
 
 graft redis.submodule

diff --git a/build_scripts/update_redis_server.py b/build_scripts/update_redis_server.py
@@ -8,7 +8,7 @@
 import tempfile
 
 
-redis_version = os.environ.get('REDIS_VERSION', '6.2.7')
+redis_version = os.environ.get('REDIS_VERSION', '6.2.9')
 url = f'http://download.redis.io/releases/redis-{redis_version}.tar.gz'
 
 
@@ -30,5 +30,5 @@
         # print('Updating jemalloc')
         # os.system('(cd redis.submodule;./deps/update-jemalloc.sh 4.0.4)')
 
-        print('Adding new redis.submodule files to git')
-        os.system('git add redis.submodule')
+        # print('Adding new redis.submodule files to git')
+        # os.system('git add redis.submodule')
diff --git a/pyproject.toml b/pyproject.toml
@@ -0,0 +1,8 @@
+# Copyright 2023, Yahoo Inc.
+# Licensed under the terms of the bsd license.  See the LICENSE file in the project root for terms
+[build-system]
+requires = [
+  "setuptools >= 40.9.0",
+  "wheel",
+]
+build-backend = "setuptools.build_meta"
diff --git a/pytest.ini b/pytest.ini
@@ -0,0 +1,7 @@
+[pytest]
+# addopts = --maxfail=5
+log_format = %(levelname)s:%(name)s:%(lineno)d:%(message)s
+log_date_format = %Y-%m-%d %H:%M:%S
+log_file_level = debug
+log_level = debug
+junit_family=xunit1
diff --git a/redislite/__init__.py b/redislite/__init__.py
@@ -66,7 +66,7 @@
     import pkg_resources
     __git_version__ = pkg_resources.get_distribution("redislite").version
     __version__ = __git_version__
-except ImportError:
+except ImportError:  # pragma: no cover
     pass
 
 

diff --git a/redislite/client.py b/redislite/client.py
@@ -72,7 +72,7 @@ def _cleanup(self, sys_modules=None):
             sys.modules.update(sys_modules)
 
         if self.pid:
-            logger.debug('Connection count: %s', self._connection_count())
+            # logger.debug('Connection count: %s', self._connection_count())
             if self._connection_count() <= 1:
                 logger.debug(
                     'Last client using the connection, shutting down the '
@@ -83,15 +83,46 @@ def _cleanup(self, sys_modules=None):
                 logger.debug(
                     'Shutting down redis server with pid of %r', self.pid
                 )
-                self.shutdown()
+                try:
+                    self.shutdown(save=True, now=True, force=True)
+                    try:  # pragma: no cover
+                        process = psutil.Process(self.pid)
+                    except psutil.NoSuchProcess:  # pragma: no cover
+                        process = None
+                    if process:   # pragma: no cover
+                        for i in range(50):
+                            if not process.is_running():
+                                break
+                            time.sleep(.2)
+                except redis.RedisError:  # pragma: no cover
+                    if self.pid != 0:
+                        try:
+                            process = psutil.Process(self.pid)
+                            if process.is_running() and process.pid != 0:
+                                logger.info(f'Redis shutdown failed, sending sigterm to {self.pid}')
+                                os.kill(self.pid, signal.SIGTERM)
+                                for i in range(120):  # default shutdown timeout is 10 seconds
+                                    if not process.is_running():
+                                        break
+                                    time.sleep(.1)
+                            if process.is_running() and process.pid != 0:
+                                logger.warning('Redis graceful shutdown failed, forcefully killing pid %r', self.pid)
+                                os.kill(self.pid, signal.SIGKILL)
+                        except psutil.NoSuchProcess:
+                            pass
                 self.socket_file = None
 
                 if self.pidfile and os.path.exists(
                         self.pidfile
                 ):   # pragma: no cover
                     # noinspection PyTypeChecker
                     pid = int(open(self.pidfile).read())
-                    os.kill(pid, signal.SIGKILL)
+                    try:
+                        process = psutil.Process(pid)
+                        if process.is_running():
+                            os.kill(pid, signal.SIGKILL)
+                    except psutil.NoSuchProcess:
+                        pass
 
                 if self.redis_dir and os.path.isdir(
                         self.redis_dir
@@ -122,8 +153,13 @@ def _connection_count(self):
         """
         if not self._is_redis_running():  # pragma: no cover
             return 0
+
         active_connections = 0
-        for client in self.client_list():
+        client_list = self.client_list()
+        if not client_list:  # pragma: no cover
+            return 0
+
+        for client in client_list:
             flags = client.get('flags', '')
             flags = flags.upper()
             if 'U' in flags or 'N' in flags:  # pragma: no cover
@@ -289,12 +325,12 @@ def _load_setting_registry(self):
             if pid_number:
                 process = psutil.Process(pid_number)
                 if not process.is_running():  # pragma: no cover
-                    logger.warn(
+                    logger.warning(
                         'Loaded registry for non-existent redis-server'
                     )
                     return
         else:  # pragma: no cover
-            logger.warn('No pidfile found')
+            logger.warning('No pidfile found')
             return
         self.pidfile = settings['pidfile']
         self.socket_file = settings['unixsocket']
@@ -487,8 +523,11 @@ def pid(self):
         return 0  # pragma: no cover
 
 
+class BaseRedis(redis.Redis):
+    pass
+
 # noinspection PyUnresolvedReferences
-class Redis(RedisMixin, redis.Redis):
+class Redis(RedisMixin, BaseRedis):
     """
     This class provides an enhanced version of the :class:`redis.Redis()` class
     that uses an embedded redis-server by default.
@@ -595,8 +634,12 @@ class are supported.
     pass
 
 
+class BaseStrictRedis(redis.StrictRedis):
+    pass
+
+
 # noinspection PyUnresolvedReferences
-class StrictRedis(RedisMixin, redis.StrictRedis):
+class StrictRedis(RedisMixin, BaseStrictRedis):
     """
     This class provides an enhanced version of the :class:`redis.StrictRedis()`
     class that uses an embedded redis-server by default.

diff --git a/screwdriver.yaml b/screwdriver.yaml
@@ -3,8 +3,8 @@ shared:
   environment:
     CHANGELOG_FILENAME: docs/source/topic/changelog.md
     PACKAGE_DIRECTORY: redislite
-    REDIS_VERSION: '6.2.7'
-    TOX_ENVLIST: py37,py38,py39,py310,py311
+    REDIS_VERSION: '6.2.9'
+    TOX_ENVLIST: py38,py39,py310,py311
   steps:
     - postinit_os: build_scripts/update_redis_server.py
 

diff --git a/setup.cfg b/setup.cfg
@@ -13,9 +13,10 @@ classifiers =
     Operating System :: POSIX
     Programming Language :: C
     Programming Language :: Python :: 3
-    Programming Language :: Python :: 3.6
-    Programming Language :: Python :: 3.7
     Programming Language :: Python :: 3.8
+    Programming Language :: Python :: 3.9
+    Programming Language :: Python :: 3.10
+    Programming Language :: Python :: 3.11
     Programming Language :: Python :: Implementation :: CPython
     Programming Language :: Python
     Topic :: Software Development :: Libraries :: Python Modules
@@ -34,16 +35,16 @@ project_urls =
     Documentation = https://redislite.readthedocs.io/en/latest/
     Source = https://github.com/yahoo/redislite
 url = https://github.com/yahoo/redislite
-version = 6.2.6
+version = 6.2.9
 
 [options]
 install_requires=
-    redis
+    redis>=4.5
     psutil
     setuptools>38.0
 packages=
     redislite
-python_requires = >=3.7.0
+python_requires = >=3.8.0
 
 [screwdrivercd.version]
 version_type = sdv4_SD_BUILD
diff --git a/tests/test_client.py b/tests/test_client.py
@@ -14,6 +14,7 @@
 import shutil
 import stat
 import tempfile
+import time
 import unittest
 from redis.exceptions import ConnectionError
 
@@ -49,9 +50,14 @@ def _redis_server_processes(self):
 
         return len(processes)
 
+    def _log_redis_log(self, connection):
+        if os.path.exists(connection.logfile):
+            with open(connection.logfile) as f:
+                print(f.read())
+
     def _log_redis_pid(self, connection):
-        logger.debug(
-            '%s: r.pid: %d', str(inspect.stack()[1][3]), connection.pid
+        print(
+            f'{str(inspect.stack()[1][3])}: r.pid: {connection.pid}'
         )
 
     def setUp(self):
@@ -67,10 +73,12 @@ def tearDown(self):
     def test_redislite_Redis(self):
         r = redislite.Redis()
         self._log_redis_pid(r)
-
-        r.set('key', 'value')
-        result = r.get('key').decode(encoding='UTF-8')
-        self.assertEqual(result, 'value')
+        r.set(b'key', b'value')
+        time.sleep(1)
+        value = r.get(b'key')
+        r.redis_log_tail(lines=0)
+        self.assertIsInstance(value, bytes)
+        self.assertEqual(value, b'value')
 
     def test_redislite_Redis_multiple(self):
         r1 = redislite.Redis()
@@ -155,6 +163,7 @@ def test_redislite_redis_custom_socket_file_local_directory(self):
         full_socket_file_name = os.path.join(os.getcwd(), socket_file_name)
         r = redislite.Redis(unix_socket_path=socket_file_name)
         self._log_redis_pid(r)
+        r.redis_log_tail()
         self.assertEqual(r.socket_file, full_socket_file_name)
         print(os.listdir('.'))
         mode = os.stat(socket_file_name).st_mode
@@ -207,10 +216,14 @@ def test_redislite_db_file_cwd_kw(self):
 
     def test_is_redis_running_no_pidfile(self):
         r = redislite.Redis()
-        self._log_redis_pid(r)
+        self.assertTrue(r._is_redis_running(), 'Redis server should be running')
+
+        print("Shutting down the server the hard way", flush=True)
         r.shutdown()
+
+        print('Checking if the server is running', flush=True)
         result = r._is_redis_running()
-        self.assertFalse(result)
+        self.assertFalse(result, 'Redis server should not be running')
 
     def test_refcount_cleanup(self):
         self.logger.debug('Setting up 2 connections to a single redis server.')
@@ -232,6 +245,7 @@ def test_refcount_cleanup(self):
         )
         r._cleanup()
 
+        self._log_redis_log(r)
         self.assertTrue(
             os.path.exists(redis_dir),
             msg='Shutting down the server removed the temporary directory'
@@ -255,7 +269,7 @@ def test_refcount_cleanup(self):
             'Second connection count is: %s', s._connection_count()
         )
         s._cleanup()
-        with self.assertRaises(psutil.NoSuchProcess):
+        with self.assertRaises(psutil.NoSuchProcess, msg=f'Redis cleanup method did not terminate pid {pid}'):
             p = psutil.Process(pid)
 
     def test_connection_count(self):

diff --git a/tox.ini b/tox.ini
@@ -1,6 +1,7 @@
 [tox]
 skip_missing_interpreters=True
-envlist = py37,py38,py39,py310,py311
+isolated_build=True
+envlist = py38,py39,py310,py311
 
 [testenv]
 changedir = {toxinidir}