From f53310f66bddece27ca00967bbfef71af01413c2 Mon Sep 17 00:00:00 2001
From: Dhruv Bhanushali <dhruv_b@live.com>
Date: Sat, 16 Dec 2023 10:25:40 +0400
Subject: [PATCH] Use pipeline and make tallying of response codes
 Redis-resilient

---
 api/api/utils/image_proxy/__init__.py   | 28 +++++++++++++++--------
 api/test/unit/utils/test_image_proxy.py | 30 +++++++++++++++++++------
 2 files changed, 42 insertions(+), 16 deletions(-)

diff --git a/api/api/utils/image_proxy/__init__.py b/api/api/utils/image_proxy/__init__.py
index 77edba8bde6..b7f85dad6ea 100644
--- a/api/api/utils/image_proxy/__init__.py
+++ b/api/api/utils/image_proxy/__init__.py
@@ -81,7 +81,7 @@ def get_request_params_for_extension(
 
 @sync_to_async
 def _tally_response(
-    tallies,
+    tallies_conn,
     media_info: MediaInfo,
     month: str,
     domain: str,
@@ -93,14 +93,24 @@ def _tally_response(
     Pulled into a separate function to help reduce overload when skimming
     the `get` function, which is complex enough as is.
     """
-    tallies.incr(f"thumbnail_response_code:{month}:{response.status}"),
-    tallies.incr(
-        f"thumbnail_response_code_by_domain:{domain}:" f"{month}:{response.status}"
-    )
-    tallies.incr(
-        f"thumbnail_response_code_by_provider:{media_info.media_provider}:"
-        f"{month}:{response.status}"
-    )
+
+    logger = parent_logger.getChild("_tally_response")
+
+    with tallies_conn.pipeline() as tallies:
+        tallies.incr(f"thumbnail_response_code:{month}:{response.status}"),
+        tallies.incr(
+            f"thumbnail_response_code_by_domain:{domain}:" f"{month}:{response.status}"
+        )
+        tallies.incr(
+            f"thumbnail_response_code_by_provider:{media_info.media_provider}:"
+            f"{month}:{response.status}"
+        )
+        try:
+            tallies.execute()
+        except ConnectionError:
+            logger.warning(
+                "Redis connect failed, thumbnail response codes not tallied."
+            )
 
 
 _UPSTREAM_TIMEOUT = aiohttp.ClientTimeout(15)
diff --git a/api/test/unit/utils/test_image_proxy.py b/api/test/unit/utils/test_image_proxy.py
index d78255404f7..e530751631c 100644
--- a/api/test/unit/utils/test_image_proxy.py
+++ b/api/test/unit/utils/test_image_proxy.py
@@ -47,6 +47,12 @@
 </svg>"""
 
 
+cache_availability_params = pytest.mark.parametrize(
+    "is_cache_reachable, cache_name",
+    [(True, "redis"), (False, "unreachable_redis")],
+)
+
+
 @pytest.fixture
 def auth_key():
     test_key = "this is a test Photon Key boop boop, let me in"
@@ -285,7 +291,11 @@ async def raise_exc(*args, **kwargs):
 
 
 @pook.on
-def test_get_successful_records_response_code(photon_get, mock_image_data, redis):
+@cache_availability_params
+def test_get_successful_records_response_code(
+    photon_get, mock_image_data, is_cache_reachable, cache_name, request, caplog
+):
+    cache = request.getfixturevalue(cache_name)
     (
         pook.get(PHOTON_URL_FOR_TEST_IMAGE)
         .params(
@@ -298,16 +308,22 @@ def test_get_successful_records_response_code(photon_get, mock_image_data, redis
         .header("Accept", "image/*")
         .reply(200)
         .body(MOCK_BODY)
-        .mock
     )
 
     photon_get(TEST_MEDIA_INFO)
     month = get_monthly_timestamp()
-    assert redis.get(f"thumbnail_response_code:{month}:200") == b"1"
-    assert (
-        redis.get(f"thumbnail_response_code_by_domain:{TEST_IMAGE_DOMAIN}:{month}:200")
-        == b"1"
-    )
+
+    keys = [
+        f"thumbnail_response_code:{month}:200",
+        f"thumbnail_response_code_by_domain:{TEST_IMAGE_DOMAIN}:{month}:200",
+    ]
+    if is_cache_reachable:
+        for key in keys:
+            assert cache.get(key) == b"1"
+    else:
+        assert (
+            "Redis connect failed, thumbnail response codes not tallied." in caplog.text
+        )
 
 
 alert_count_params = pytest.mark.parametrize(