refactor: simplify enketo Redis client — remove connection pooling and fix pylint issues

Remove manual ConnectionPool management (redis.Redis handles pooling
internally) and create a fresh client per call. Narrow broad exception
catches to specific Redis/OS errors and eliminate global statement.

Author: FrankApiyo

.github/workflows/ci.yml

@@ -1,6 +1,6 @@
 ---
 name: CI
-on:  # yamllint disable-line rule:truthy
+on: # yamllint disable-line rule:truthy
   pull_request:
     branches: ["*"]
   push:
@@ -66,17 +66,17 @@ jobs:
       matrix:
         test_path:
           - [
-            " Django Unit Tests (Libraries, Main, RestServices, SMS Support, Viewer, Messaging)",
-            "python manage.py test onadata/libs onadata/apps/main onadata/apps/restservice onadata/apps/sms_support onadata/apps/viewer onadata/apps/messaging --noinput --timing --settings=onadata.settings.github_actions_test --verbosity=2 --parallel=4",
-          ]
+              " Django Unit Tests (Libraries, Main, RestServices, SMS Support, Viewer, Messaging)",
+              "python manage.py test onadata/libs onadata/apps/main onadata/apps/restservice onadata/apps/sms_support onadata/apps/viewer onadata/apps/messaging --noinput --timing --settings=onadata.settings.github_actions_test --verbosity=2 --parallel=4",
+            ]
           - [
-            "Django Unit Tests API",
-            "python manage.py test onadata/apps/api --noinput --timing --settings=onadata.settings.github_actions_test --verbosity=2 --parallel=4",
-          ]
+              "Django Unit Tests API",
+              "python manage.py test onadata/apps/api --noinput --timing --settings=onadata.settings.github_actions_test --verbosity=2 --parallel=4",
+            ]
           - [
-            "Django Unit Tests Logger",
-            "python manage.py test onadata/apps/logger --noinput --timing --settings=onadata.settings.github_actions_test --verbosity=2 --parallel=4",
-          ]
+              "Django Unit Tests Logger",
+              "python manage.py test onadata/apps/logger --noinput --timing --settings=onadata.settings.github_actions_test --verbosity=2 --parallel=4",
+            ]
     name: "${{ matrix.test_path[0] }}"
     runs-on: ubuntu-22.04
     needs: static-analysis
@@ -184,7 +184,7 @@ jobs:
             optional_packages=PyYAML django-redis ${{ secrets.ECR_OPTIONAL_PACKAGES }}
 
       - name: Install Trivy
-        uses: aquasecurity/setup-trivy@v0.2.4
+        uses: aquasecurity/setup-trivy@v0.2.6
         if: github.event_name == 'pull_request' || github.event_name == 'push'
         with:
           version: v0.69.1

onadata/apps/api/viewsets/xform_viewset.py

@@ -531,7 +531,6 @@ def enketo_links(self, request, **kwargs):
         self.object = self.get_object()
         show_preview = request.GET.get("show_preview") == "true"
 
-        # --- try cache first ---
         if show_preview:
             cached = get_cached_preview_url(self.object.pk)
             if cached:
@@ -541,13 +540,11 @@ def enketo_links(self, request, **kwargs):
             if cached:
                 return Response(self._build_survey_response(cached))
 
-        # --- cache miss: call Enketo API ---
         result = self._fetch_enketo_urls(request)
         if isinstance(result, Response):
             return result
         enketo_urls = result
 
-        # --- store in cache ---
         if show_preview:
             preview_url = enketo_urls.get("preview_url", "")
             if preview_url:

onadata/libs/utils/enketo_redis.py

@@ -29,32 +29,17 @@
 # Default TTL: 24 hours.  Override via ENKETO_LINKS_CACHE_TTL (seconds).
 DEFAULT_CACHE_TTL = 86400
 
-# Module-level connection pool — shared across all requests.
-_pool = None
 
-
-def _get_pool():
-    """Return the shared ``ConnectionPool``, creating it on first call."""
-    global _pool  # noqa: PLW0603
-    if _pool is not None:
-        return _pool
+def _get_client():
+    """Return a new Redis client, or *None* if not configured."""
     url = getattr(settings, "ENKETO_LINKS_REDIS_URL", None)
     if not url:
         return None
     try:
-        _pool = redis.ConnectionPool.from_url(url, decode_responses=True)
-        return _pool
-    except Exception:
-        logger.exception("Failed to create enketo-links Redis connection pool")
-        return None
-
-
-def _get_connection():
-    """Return a Redis client backed by the shared connection pool, or *None*."""
-    pool = _get_pool()
-    if pool is None:
+        return redis.Redis.from_url(url, decode_responses=True)
+    except (redis.ConnectionError, redis.RedisError, OSError):
+        logger.exception("Failed to create enketo-links Redis client")
         return None
-    return redis.Redis(connection_pool=pool)
 
 
 def _cache_ttl():
@@ -68,13 +53,13 @@ def get_cached_survey_urls(form_pk):
     ``preview_url``, ``single_once_url``) or ``None`` on cache miss /
     error.
     """
-    conn = _get_connection()
-    if conn is None:
+    client = _get_client()
+    if client is None:
         return None
     try:
-        data = conn.hgetall(f"{SURVEY_URLS_PREFIX}{int(form_pk)}")
+        data = client.hgetall(f"{SURVEY_URLS_PREFIX}{int(form_pk)}")
         return data if data else None
-    except Exception:
+    except (redis.ConnectionError, redis.RedisError, OSError):
         logger.exception("enketo-links Redis read error")
         return None
 
@@ -85,43 +70,43 @@ def store_survey_urls(form_pk, urls):
     *urls* is a dict as returned by the Enketo API (keys like
     ``offline_url``, ``preview_url``, ``single_once_url``).
     """
-    conn = _get_connection()
-    if conn is None:
+    client = _get_client()
+    if client is None:
         return
     try:
         key = f"{SURVEY_URLS_PREFIX}{int(form_pk)}"
         mapping = {k: str(v) for k, v in urls.items() if v is not None}
         if mapping:
-            pipe = conn.pipeline()
+            pipe = client.pipeline()
             pipe.hset(key, mapping=mapping)
             pipe.expire(key, _cache_ttl())
             pipe.execute()
-    except Exception:
+    except (redis.ConnectionError, redis.RedisError, OSError):
         logger.exception("enketo-links Redis write error")
 
 
 def get_cached_preview_url(form_pk):
     """Read the cached preview URL for *form_pk*, or ``None``."""
-    conn = _get_connection()
-    if conn is None:
+    client = _get_client()
+    if client is None:
         return None
     try:
-        return conn.get(f"{PREVIEW_URL_PREFIX}{int(form_pk)}")
-    except Exception:
+        return client.get(f"{PREVIEW_URL_PREFIX}{int(form_pk)}")
+    except (redis.ConnectionError, redis.RedisError, OSError):
         logger.exception("enketo-links Redis read error")
         return None
 
 
 def store_preview_url(form_pk, url):
     """Cache just the preview URL for *form_pk*."""
-    conn = _get_connection()
-    if conn is None:
+    client = _get_client()
+    if client is None:
         return
     try:
-        conn.set(
+        client.set(
             f"{PREVIEW_URL_PREFIX}{int(form_pk)}",
             str(url),
             ex=_cache_ttl(),
         )
-    except Exception:
+    except (redis.ConnectionError, redis.RedisError, OSError):
         logger.exception("enketo-links Redis write error")

onadata/libs/utils/tests/test_enketo_redis.py

@@ -2,6 +2,7 @@
 
 from unittest.mock import MagicMock, patch
 
+import redis
 from django.test import TestCase, override_settings
 
 from onadata.libs.utils import enketo_redis
@@ -15,20 +16,9 @@
 )
 
 
-def _reset_pool():
-    """Reset the module-level connection pool between tests."""
-    enketo_redis._pool = None
-
-
 class EnketoRedisDisabledTest(TestCase):
     """When ENKETO_LINKS_REDIS_URL is empty, all functions are no-ops."""
 
-    def setUp(self):
-        _reset_pool()
-
-    def tearDown(self):
-        _reset_pool()
-
     @override_settings(ENKETO_LINKS_REDIS_URL="")
     def test_get_cached_survey_urls_returns_none(self):
         self.assertIsNone(get_cached_survey_urls(42))
@@ -51,24 +41,16 @@ class EnketoRedisCacheTest(TestCase):
     """Test caching with a mocked Redis connection."""
 
     def setUp(self):
-        _reset_pool()
         self.mock_redis = MagicMock()
-        self.pool_patcher = patch.object(
+        self.client_patcher = patch.object(
             enketo_redis,
-            "_get_pool",
-            return_value=MagicMock(),
-        )
-        self.redis_patcher = patch(
-            "onadata.libs.utils.enketo_redis.redis.Redis",
+            "_get_client",
             return_value=self.mock_redis,
         )
-        self.pool_patcher.start()
-        self.redis_patcher.start()
+        self.client_patcher.start()
 
     def tearDown(self):
-        self.pool_patcher.stop()
-        self.redis_patcher.stop()
-        _reset_pool()
+        self.client_patcher.stop()
 
     # --- survey URLs ---
 
@@ -89,7 +71,7 @@ def test_get_cached_survey_urls_returns_none_on_miss(self):
         self.assertIsNone(get_cached_survey_urls(99))
 
     def test_get_cached_survey_urls_returns_none_on_error(self):
-        self.mock_redis.hgetall.side_effect = Exception("boom")
+        self.mock_redis.hgetall.side_effect = redis.RedisError("boom")
         self.assertIsNone(get_cached_survey_urls(99))
 
     def test_store_survey_urls_writes_hash_with_ttl(self):

onadata/libs/utils/viewer_tools.py

@@ -297,6 +297,9 @@ def get_form_url(
     provided then Enketo will request the form list from
     https://example.com/[username]/formList. Same applies for preview if
     preview is True and also to a single form when xform_pk is provided.
+
+    When *protocol* is ``None`` (the default) it is read from the
+    ``ENKETO_PROTOCOL`` Django setting, falling back to ``"https"``.
     """
     if protocol is None:
         protocol = getattr(settings, "ENKETO_PROTOCOL", "https")