From cdc929d95e86e6fa126c66a31244305980efa26c Mon Sep 17 00:00:00 2001 From: Dhruv Bhanushali Date: Sat, 16 Dec 2023 10:30:51 +0400 Subject: [PATCH] Make caching of image extension Redis-resilient --- api/api/utils/image_proxy/extension.py | 23 ++++++++++++++++++++--- api/test/unit/utils/test_image_proxy.py | 24 +++++++++++++++++++++--- 2 files changed, 41 insertions(+), 6 deletions(-) diff --git a/api/api/utils/image_proxy/extension.py b/api/api/utils/image_proxy/extension.py index 2f2612022c3..4c6537b34e1 100644 --- a/api/api/utils/image_proxy/extension.py +++ b/api/api/utils/image_proxy/extension.py @@ -1,3 +1,4 @@ +import logging from os.path import splitext from urllib.parse import urlparse @@ -5,16 +6,23 @@ import django_redis import sentry_sdk from asgiref.sync import sync_to_async +from redis.exceptions import ConnectionError from api.utils.aiohttp import get_aiohttp_session from api.utils.asyncio import do_not_wait_for from api.utils.image_proxy.exception import UpstreamThumbnailException +parent_logger = logging.getLogger(__name__) + + _HEAD_TIMEOUT = aiohttp.ClientTimeout(10) async def get_image_extension(image_url: str, media_identifier) -> str | None: + logger = parent_logger.getChild("get_image_extension") + is_redis_reachable = True + cache = django_redis.get_redis_connection("default") key = f"media:{media_identifier}:thumb_type" @@ -22,8 +30,12 @@ async def get_image_extension(image_url: str, media_identifier) -> str | None: if not ext: # If the extension is not present in the URL, try to get it from the redis cache - ext = await sync_to_async(cache.get)(key) - ext = ext.decode("utf-8") if ext else None + try: + ext = await sync_to_async(cache.get)(key) + ext = ext.decode("utf-8") if ext else None + except ConnectionError: + logger.warning("Redis connect failed, cannot get cached image extension.") + is_redis_reachable = False if not ext: # If the extension is still not present, try getting it from the content type @@ -37,7 +49,12 @@ async def get_image_extension(image_url: str, media_identifier) -> str | None: else: ext = None - do_not_wait_for(sync_to_async(cache.set)(key, ext if ext else "unknown")) + if is_redis_reachable: + do_not_wait_for( + sync_to_async(cache.set)(key, ext if ext else "unknown") + ) + else: + logger.warning("Redis connect failed, cannot cache image extension.") except Exception as exc: sentry_sdk.capture_exception(exc) raise UpstreamThumbnailException( diff --git a/api/test/unit/utils/test_image_proxy.py b/api/test/unit/utils/test_image_proxy.py index e530751631c..cc9983736f1 100644 --- a/api/test/unit/utils/test_image_proxy.py +++ b/api/test/unit/utils/test_image_proxy.py @@ -529,9 +529,18 @@ def test_photon_get_raises_by_not_allowed_types(photon_get, image_type): ({"Content-Type": "unknown"}, b"unknown"), ], ) +@cache_availability_params def test_photon_get_saves_image_type_to_cache( - photon_get, redis, headers, expected_cache_val + photon_get, + headers, + expected_cache_val, + is_cache_reachable, + cache_name, + request, + caplog, ): + cache = request.getfixturevalue(cache_name) + image_url = TEST_IMAGE_URL.replace(".jpg", "") image = ImageFactory.create(url=image_url) media_info = MediaInfo( @@ -544,5 +553,14 @@ def test_photon_get_saves_image_type_to_cache( with pytest.raises(UnsupportedMediaType): photon_get(media_info) - key = f"media:{image.identifier}:thumb_type" - assert redis.get(key) == expected_cache_val + key = f"media:{image.identifier}:thumb_type" + if is_cache_reachable: + assert cache.get(key) == expected_cache_val + else: + assert all( + message in caplog.text + for message in [ + "Redis connect failed, cannot get cached image extension.", + "Redis connect failed, cannot cache image extension.", + ] + )