From ebc08ad2080b85d4ced867dc24ab5402f2984c96 Mon Sep 17 00:00:00 2001 From: Zeid Zabaneh Date: Fri, 15 Dec 2023 12:16:18 -0500 Subject: [PATCH] settings: add cache configuration (bug 1870285) - add fallback DB backend - add environment based redis configuration - remove code that is no longer needed --- src/lando/api/legacy/cache.py | 64 -------------------------- src/lando/api/tests/conftest.py | 22 --------- src/lando/api/tests/test_dockerflow.py | 4 +- src/lando/api/tests/test_health.py | 21 --------- src/lando/main/models/configuration.py | 6 +-- src/lando/settings.py | 21 +++++++++ 6 files changed, 25 insertions(+), 113 deletions(-) delete mode 100644 src/lando/api/legacy/cache.py diff --git a/src/lando/api/legacy/cache.py b/src/lando/api/legacy/cache.py deleted file mode 100644 index 6d8053b9..00000000 --- a/src/lando/api/legacy/cache.py +++ /dev/null @@ -1,64 +0,0 @@ -# This Source Code Form is subject to the terms of the Mozilla Public -# License, v. 2.0. If a copy of the MPL was not distributed with this -# file, You can obtain one at http://mozilla.org/MPL/2.0/. - -from __future__ import annotations - -import logging - -from flask_caching import Cache -from flask_caching.backends.rediscache import RedisCache -from redis import RedisError - -from landoapi.redis import SuppressRedisFailure -from landoapi.systems import Subsystem - -# 60s * 60m * 24h -DEFAULT_CACHE_KEY_TIMEOUT_SECONDS = 60 * 60 * 24 - -logger = logging.getLogger(__name__) -cache = Cache() -cache.suppress_failure = SuppressRedisFailure - - -class CacheSubsystem(Subsystem): - name = "cache" - - def init_app(self, app): - super().init_app(app) - host = self.flask_app.config.get("CACHE_REDIS_HOST") - if self.flask_app.config.get("CACHE_DISABLED"): - # Default to not caching for testing. - logger.warning("Cache initialized in null mode.") - cache_config = {"CACHE_TYPE": "NullCache"} - elif not host: - logger.warning("Cache initialized in filesystem mode.") - cache_config = {"CACHE_TYPE": "FileSystemCache", "CACHE_DIR": "/tmp/cache"} - else: - cache_config = {"CACHE_TYPE": "redis", "CACHE_REDIS_HOST": host} - config_keys = ("CACHE_REDIS_PORT", "CACHE_REDIS_PASSWORD", "CACHE_REDIS_DB") - for k in config_keys: - v = self.flask_app.config.get(k) - if v is not None: - cache_config[k] = v - - cache.init_app(self.flask_app, config=cache_config) - - def healthy(self) -> bool | str: - if not isinstance(cache.cache, RedisCache): - return "Cache is not configured to use redis" - - # Dirty, but if this breaks in the future we can instead - # create our own redis-py client with its own connection - # pool. - redis = cache.cache._read_client - - try: - redis.ping() - except RedisError as exc: - return "RedisError: {!s}".format(exc) - - return True - - -cache_subsystem = CacheSubsystem() diff --git a/src/lando/api/tests/conftest.py b/src/lando/api/tests/conftest.py index d999186c..0683d8a9 100644 --- a/src/lando/api/tests/conftest.py +++ b/src/lando/api/tests/conftest.py @@ -11,7 +11,6 @@ import flask.testing import pytest -import redis import requests import requests_mock import sqlalchemy @@ -19,7 +18,6 @@ from pytest_flask.plugin import JSONResponse from landoapi.app import SUBSYSTEMS, construct_app, load_config -from landoapi.cache import cache from landoapi.mocks.auth import TEST_JWKS, MockAuth0 from landoapi.phabricator import PhabricatorClient from landoapi.projects import ( @@ -159,9 +157,6 @@ def docker_env_vars(versionfile, monkeypatch): monkeypatch.setenv("BUGZILLA_URL", "asdfasdfasdfasdfasdfasdf") monkeypatch.setenv("OIDC_IDENTIFIER", "lando-api") monkeypatch.setenv("OIDC_DOMAIN", "lando-api.auth0.test") - # Explicitly shut off cache use for all tests. Tests can re-enable the cache - # with the redis_cache fixture. - monkeypatch.delenv("CACHE_REDIS_HOST", raising=False) monkeypatch.delenv("CSP_REPORTING_URL", raising=False) # Don't suppress email in tests, but point at localhost so that any # real attempt would fail. @@ -362,23 +357,6 @@ def get_client(api_key=None): return get_client -@pytest.fixture -def redis_cache(app): - cache.init_app( - app, config={"CACHE_TYPE": "redis", "CACHE_REDIS_HOST": "redis.cache"} - ) - try: - cache.clear() - except redis.exceptions.ConnectionError: - if EXTERNAL_SERVICES_SHOULD_BE_PRESENT: - raise - else: - pytest.skip("Could not connect to Redis") - yield cache - cache.clear() - cache.init_app(app, config={"CACHE_TYPE": "null", "CACHE_NO_NULL_WARNING": True}) - - @pytest.fixture def celery_app(app): """Configure our app's Celery instance for use with the celery_worker fixture.""" diff --git a/src/lando/api/tests/test_dockerflow.py b/src/lando/api/tests/test_dockerflow.py index d3242037..aa7ca53b 100644 --- a/src/lando/api/tests/test_dockerflow.py +++ b/src/lando/api/tests/test_dockerflow.py @@ -23,13 +23,13 @@ def test_dockerflow_version_matches_disk_contents(client, versionfile): def test_heartbeat_returns_200( - client, db, phabdouble, request_mocker, redis_cache, jwks, treestatusdouble + client, db, phabdouble, request_mocker, jwks, treestatusdouble ): assert client.get("/__heartbeat__").status_code == 200 def test_heartbeat_returns_http_502_if_phabricator_ping_returns_error( - client, request_mocker, redis_cache, jwks, treestatusdouble + client, request_mocker, jwks, treestatusdouble ): error_json = { "result": None, diff --git a/src/lando/api/tests/test_health.py b/src/lando/api/tests/test_health.py index cde7456f..b7a53f31 100644 --- a/src/lando/api/tests/test_health.py +++ b/src/lando/api/tests/test_health.py @@ -4,11 +4,9 @@ from unittest.mock import Mock -import redis from sqlalchemy.exc import SQLAlchemyError from landoapi.auth import auth0_subsystem -from landoapi.cache import cache_subsystem from landoapi.phabricator import PhabricatorAPIException, phabricator_subsystem from landoapi.storage import db_subsystem @@ -37,25 +35,6 @@ def raises(*args, **kwargs): assert phabricator_subsystem.healthy() is not True -def test_cache_healthy(redis_cache): - assert cache_subsystem.healthy() is True - - -def test_cache_unhealthy_configuration(): - assert cache_subsystem.healthy() is not True - - -def test_cache_unhealthy_service(redis_cache, monkeypatch): - mock_cache = Mock(redis_cache) - mock_cache.cache._read_client.ping.side_effect = redis.TimeoutError - monkeypatch.setattr("landoapi.cache.cache", mock_cache) - monkeypatch.setattr("landoapi.cache.RedisCache", type(mock_cache.cache)) - - health = cache_subsystem.healthy() - assert health is not True - assert health.startswith("RedisError:") - - def test_auth0_healthy(app, jwks): assert auth0_subsystem.healthy() is True diff --git a/src/lando/main/models/configuration.py b/src/lando/main/models/configuration.py index a9c5de14..e8b3bd04 100644 --- a/src/lando/main/models/configuration.py +++ b/src/lando/main/models/configuration.py @@ -8,7 +8,6 @@ Union, ) -from landoapi.cache import cache from landoapi.models.base import Base from landoapi.storage import db @@ -71,7 +70,6 @@ def value(self) -> ConfigurationValueType: raise ValueError("Could not parse raw value for configuration variable.") @classmethod - @cache.memoize() def get( cls, key: ConfigurationKey, default: ConfigurationValueType ) -> ConfigurationValueType: @@ -114,13 +112,11 @@ def set( logger.info(f"Creating new configuration variable {key.value}.") record = cls() - logger.info("Deleting memoized cache for configuration variables.") if record.raw_value: logger.info( f"Configuration variable {key.value} previously set to {record.raw_value} " f"({record.value})" ) - cache.delete_memoized(cls.get) record.variable_type = variable_type record.key = key.value record.raw_value = raw_value @@ -129,4 +125,6 @@ def set( logger.info( f"Configuration variable {key.value} set to {raw_value} ({record.value})" ) + + # TODO: add caching return record diff --git a/src/lando/settings.py b/src/lando/settings.py index b761c596..5b2b7602 100644 --- a/src/lando/settings.py +++ b/src/lando/settings.py @@ -94,6 +94,27 @@ } } +REDIS_CONFIG = { + "HOST": os.getenv("REDIS_HOST"), + "PORT": os.getenv("REDIS_PORT"), + "USER": os.getenv("REDIS_USER"), + "PASSWORD": os.getenv("REDIS_PASSWORD"), + } + +CACHES = {} + +if all(REDIS_CONFIG.values()): + CACHES["default"] = { + "BACKEND": "django.core.cache.backends.redis.RedisCache", + "LOCATION": [ + "redis://{USER}:{PASSWORD}@{HOST}:{PORT}".format(**REDIS_CONFIG), + ], + } +else: + CACHES["default"] = { + "BACKEND": "django.core.cache.backends.db.DatabaseCache", + "LOCATION": "lando_cache", + } # Password validation # https://docs.djangoproject.com/en/dev/ref/settings/#auth-password-validators