Skip to content

Commit

Permalink
Fix unhandled Redis exception in the /api/server/health/ endpoint
Browse files Browse the repository at this point in the history
`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.
  • Loading branch information
SpecLad committed Feb 1, 2024
1 parent 3db7be6 commit c576bb4
Show file tree
Hide file tree
Showing 7 changed files with 10 additions and 36 deletions.
4 changes: 4 additions & 0 deletions changelog.d/20240131_202605_roman_bump_health_check.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
### Fixed

- Fix Redis exceptions crashing the `/api/server/health/` endpoint
(<https://github.com/opencv/cvat/pull/7417>)
6 changes: 1 addition & 5 deletions cvat/apps/health/apps.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,16 +3,12 @@
# SPDX-License-Identifier: MIT

from django.apps import AppConfig
from django.conf import settings

from health_check.plugins import plugin_dir

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)
27 changes: 0 additions & 27 deletions cvat/apps/health/backends.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -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)))
2 changes: 1 addition & 1 deletion cvat/requirements/base.in
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions cvat/requirements/base.txt
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
# SHA1:97dd55235acd1b8766b6bd1227ae6d776e9d8e7a
# SHA1:07743309d7b390659b762ca30db20ebc07ac81bc
#
# This file is autogenerated by pip-compile-multi
# To update, run:
Expand Down Expand Up @@ -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
Expand Down
1 change: 1 addition & 0 deletions cvat/settings/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down
2 changes: 1 addition & 1 deletion utils/dataset_manifest/requirements.txt
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
# SHA1:2c4fe23872675e963864abe27e1644f42865f712
# SHA1:c60d1ed19f53b618c5528cd4b4fc708bc7ba404f
#
# This file is autogenerated by pip-compile-multi
# To update, run:
Expand Down

0 comments on commit c576bb4

Please sign in to comment.