From c576bb4d8108845fdeb1c0b9a4407ab8179ead01 Mon Sep 17 00:00:00 2001 From: Roman Donchenko Date: Mon, 29 Jan 2024 20:48:21 +0200 Subject: [PATCH] Fix unhandled Redis exception in the /api/server/health/ endpoint `CustomCacheBackend` does not handle `RedisError`, so if Redis is not responding, the backend will propagate the exception, and the endpoint will crash with a 500 response code. This problem has been fixed upstream in django-health-check 3.18, so just update to the latest version and remove our custom cache backend. The upstream version, unlike ours, doesn't handle `sqlite3.DatabaseError`, but we're not using SQLite for caching anymore, so I don't think it matters. --- ...20240131_202605_roman_bump_health_check.md | 4 +++ cvat/apps/health/apps.py | 6 +---- cvat/apps/health/backends.py | 27 ------------------- cvat/requirements/base.in | 2 +- cvat/requirements/base.txt | 4 +-- cvat/settings/base.py | 1 + utils/dataset_manifest/requirements.txt | 2 +- 7 files changed, 10 insertions(+), 36 deletions(-) create mode 100644 changelog.d/20240131_202605_roman_bump_health_check.md diff --git a/changelog.d/20240131_202605_roman_bump_health_check.md b/changelog.d/20240131_202605_roman_bump_health_check.md new file mode 100644 index 000000000000..e2ce514d285c --- /dev/null +++ b/changelog.d/20240131_202605_roman_bump_health_check.md @@ -0,0 +1,4 @@ +### Fixed + +- Fix Redis exceptions crashing the `/api/server/health/` endpoint + () diff --git a/cvat/apps/health/apps.py b/cvat/apps/health/apps.py index 986f7e342588..a457048b87c9 100644 --- a/cvat/apps/health/apps.py +++ b/cvat/apps/health/apps.py @@ -3,7 +3,6 @@ # SPDX-License-Identifier: MIT from django.apps import AppConfig -from django.conf import settings from health_check.plugins import plugin_dir @@ -11,8 +10,5 @@ class HealthConfig(AppConfig): name = 'cvat.apps.health' def ready(self): - from .backends import OPAHealthCheck, CustomCacheBackend + from .backends import OPAHealthCheck plugin_dir.register(OPAHealthCheck) - - for backend in settings.CACHES: - plugin_dir.register(CustomCacheBackend, backend=backend) diff --git a/cvat/apps/health/backends.py b/cvat/apps/health/backends.py index df0e1f891d36..2f361117173a 100644 --- a/cvat/apps/health/backends.py +++ b/cvat/apps/health/backends.py @@ -3,14 +3,11 @@ # SPDX-License-Identifier: MIT import requests -import sqlite3 from health_check.backends import BaseHealthCheckBackend from health_check.exceptions import HealthCheckException -from health_check.exceptions import ServiceReturnedUnexpectedResult, ServiceUnavailable from django.conf import settings -from django.core.cache import CacheKeyWarning, caches from cvat.utils.http import make_requests_session @@ -28,27 +25,3 @@ def check_status(self): def identifier(self): return self.__class__.__name__ - -class CustomCacheBackend(BaseHealthCheckBackend): - def __init__(self, backend="default"): - super().__init__() - self.backend = backend - - def identifier(self): - return f"Cache backend: {self.backend}" - - def check_status(self): - try: - cache = caches[self.backend] - - cache.set("djangohealtcheck_test", "itworks") - if not cache.get("djangohealtcheck_test") == "itworks": - raise ServiceUnavailable("Cache key does not match") - except CacheKeyWarning as e: - self.add_error(ServiceReturnedUnexpectedResult("Cache key warning"), e) - except ValueError as e: - self.add_error(ServiceReturnedUnexpectedResult("ValueError"), e) - except ConnectionError as e: - self.add_error(ServiceReturnedUnexpectedResult("Connection Error"), e) - except sqlite3.DatabaseError as e: - raise ServiceUnavailable("Cache error: {}".format(str(e))) diff --git a/cvat/requirements/base.in b/cvat/requirements/base.in index 712e4f7e44fb..5b685b2842a1 100644 --- a/cvat/requirements/base.in +++ b/cvat/requirements/base.in @@ -22,7 +22,7 @@ django-compressor==4.3.1 django-cors-headers==3.5.0 django-crum==0.7.9 django-filter==2.4.0 -django-health-check==3.17.0 +django-health-check>=3.18.1,<4 django-rq==2.8.1 django-sendfile2==0.7.0 Django~=4.2.1 diff --git a/cvat/requirements/base.txt b/cvat/requirements/base.txt index c8b58f0a3c86..a1e2999955e4 100644 --- a/cvat/requirements/base.txt +++ b/cvat/requirements/base.txt @@ -1,4 +1,4 @@ -# SHA1:97dd55235acd1b8766b6bd1227ae6d776e9d8e7a +# SHA1:07743309d7b390659b762ca30db20ebc07ac81bc # # This file is autogenerated by pip-compile-multi # To update, run: @@ -105,7 +105,7 @@ django-crum==0.7.9 # via -r cvat/requirements/base.in django-filter==2.4.0 # via -r cvat/requirements/base.in -django-health-check==3.17.0 +django-health-check==3.18.1 # via -r cvat/requirements/base.in django-rq==2.8.1 # via -r cvat/requirements/base.in diff --git a/cvat/settings/base.py b/cvat/settings/base.py index e5b4012fae7a..b4a588202504 100644 --- a/cvat/settings/base.py +++ b/cvat/settings/base.py @@ -103,6 +103,7 @@ def generate_secret_key(): 'corsheaders', 'allauth.socialaccount', 'health_check', + 'health_check.cache', 'health_check.db', 'health_check.contrib.migrations', 'health_check.contrib.psutil', diff --git a/utils/dataset_manifest/requirements.txt b/utils/dataset_manifest/requirements.txt index 5c2ebd51808a..f5a34a49dc7c 100644 --- a/utils/dataset_manifest/requirements.txt +++ b/utils/dataset_manifest/requirements.txt @@ -1,4 +1,4 @@ -# SHA1:2c4fe23872675e963864abe27e1644f42865f712 +# SHA1:c60d1ed19f53b618c5528cd4b4fc708bc7ba404f # # This file is autogenerated by pip-compile-multi # To update, run: