From 744e0a070bda7e0c41eb4731d66dc648f9273a0a Mon Sep 17 00:00:00 2001 From: John Bodley <4567245+john-bodley@users.noreply.github.com> Date: Tue, 15 Sep 2020 12:48:19 -0700 Subject: [PATCH] chore: Using cache factory method (#10887) Co-authored-by: John Bodley --- UPDATING.md | 2 ++ docs/src/pages/docs/installation/caching.mdx | 18 +------------ superset/__init__.py | 2 +- superset/utils/cache_manager.py | 28 +++++--------------- tests/superset_test_config_thumbnails.py | 23 ++++++---------- tests/utils_tests.py | 28 -------------------- 6 files changed, 18 insertions(+), 83 deletions(-) diff --git a/UPDATING.md b/UPDATING.md index b533ed7ebe360..ec7b5e8faa53d 100644 --- a/UPDATING.md +++ b/UPDATING.md @@ -23,6 +23,8 @@ assists people when migrating to a new version. ## Next +* [10887](https://github.com/apache/incubator-superset/pull/10887): Breaking change: The custom cache backend changed in order to support the Flask-Caching factory method approach and thus must be registered as a custom type. See [here](https://flask-caching.readthedocs.io/en/latest/#custom-cache-backends) for specifics. + * [10794](https://github.com/apache/incubator-superset/pull/10794): Breaking change: `uuid` python package is not supported on Jinja2 anymore, only uuid functions are exposed eg: `uuid1`, `uuid3`, `uuid4`, `uuid5`. * [10674](https://github.com/apache/incubator-superset/pull/10674): Breaking change: PUBLIC_ROLE_LIKE_GAMMA was removed is favour of the new PUBLIC_ROLE_LIKE so it can be set it whatever role you want. diff --git a/docs/src/pages/docs/installation/caching.mdx b/docs/src/pages/docs/installation/caching.mdx index 266f3a06a9a92..8dd06dbaa90fa 100644 --- a/docs/src/pages/docs/installation/caching.mdx +++ b/docs/src/pages/docs/installation/caching.mdx @@ -34,23 +34,7 @@ CACHE_CONFIG = { } ``` -It is also possible to pass a custom cache initialization function in the config to handle -additional caching use cases. The function must return an object that is compatible with the -[Flask-Cache API](https://pythonhosted.org/Flask-Cache/). - -```python -from custom_caching import CustomCache - -def init_cache(app): - """Takes an app instance and returns a custom cache backend""" - config = { - 'CACHE_DEFAULT_TIMEOUT': 60 * 60 * 24, # 1 day default (in secs) - 'CACHE_KEY_PREFIX': 'superset_results', - } - return CustomCache(app, config) - -CACHE_CONFIG = init_cache -``` +Custom cache backends are also supported. See [here](https://flask-caching.readthedocs.io/en/latest/#custom-cache-backends) for specifics. Superset has a Celery task that will periodically warm up the cache based on different strategies. To use it, add the following to the `CELERYBEAT_SCHEDULE` section in `config.py`: diff --git a/superset/__init__.py b/superset/__init__.py index 3e26c3fb74b45..2d76afd5284bd 100644 --- a/superset/__init__.py +++ b/superset/__init__.py @@ -40,7 +40,7 @@ # then initialize it in app.create_app(). These fields will be removed # in subsequent PRs as things are migrated towards the factory pattern app: Flask = current_app -cache = LocalProxy(lambda: cache_manager.cache) +cache = cache_manager.cache conf = LocalProxy(lambda: current_app.config) get_feature_flags = feature_flag_manager.get_feature_flags get_manifest_files = manifest_processor.get_manifest_files diff --git a/superset/utils/cache_manager.py b/superset/utils/cache_manager.py index 4a625ea6d00bb..77b1c9b46fe4d 100644 --- a/superset/utils/cache_manager.py +++ b/superset/utils/cache_manager.py @@ -17,35 +17,19 @@ from flask import Flask from flask_caching import Cache -from superset.typing import CacheConfig - class CacheManager: def __init__(self) -> None: super().__init__() - self._tables_cache = None - self._cache = None - self._thumbnail_cache = None + self._cache = Cache() + self._tables_cache = Cache() + self._thumbnail_cache = Cache() def init_app(self, app: Flask) -> None: - self._cache = self._setup_cache(app, app.config["CACHE_CONFIG"]) - self._tables_cache = self._setup_cache( - app, app.config["TABLE_NAMES_CACHE_CONFIG"] - ) - self._thumbnail_cache = self._setup_cache( - app, app.config["THUMBNAIL_CACHE_CONFIG"] - ) - - @staticmethod - def _setup_cache(app: Flask, cache_config: CacheConfig) -> Cache: - """Setup the flask-cache on a flask app""" - if isinstance(cache_config, dict): - return Cache(app, config=cache_config) - - # Accepts a custom cache initialization function, returning an object compatible - # with Flask-Caching API. - return cache_config(app) + self._cache.init_app(app, app.config["CACHE_CONFIG"]) + self._tables_cache.init_app(app, app.config["TABLE_NAMES_CACHE_CONFIG"]) + self._thumbnail_cache.init_app(app, app.config["THUMBNAIL_CACHE_CONFIG"]) @property def tables_cache(self) -> Cache: diff --git a/tests/superset_test_config_thumbnails.py b/tests/superset_test_config_thumbnails.py index af30af4e4d61f..ddd59987ee732 100644 --- a/tests/superset_test_config_thumbnails.py +++ b/tests/superset_test_config_thumbnails.py @@ -17,9 +17,6 @@ # type: ignore from copy import copy -from cachelib.redis import RedisCache -from flask import Flask - from superset.config import * AUTH_USER_REGISTRATION_ROLE = "alpha" @@ -79,15 +76,11 @@ class CeleryConfig(object): "THUMBNAILS_SQLA_LISTENERS": False, } - -def init_thumbnail_cache(app: Flask) -> RedisCache: - return RedisCache( - host=REDIS_HOST, - port=REDIS_PORT, - db=REDIS_CELERY_DB, - key_prefix="superset_thumbnails_", - default_timeout=10000, - ) - - -THUMBNAIL_CACHE_CONFIG = init_thumbnail_cache +THUMBNAIL_CACHE_CONFIG = { + "CACHE_TYPE": "redis", + "CACHE_DEFAULT_TIMEOUT": 10000, + "CACHE_KEY_PREFIX": "superset_thumbnails_", + "CACHE_REDIS_HOST": REDIS_HOST, + "CACHE_REDIS_PORT": REDIS_PORT, + "CACHE_REDIS_DB": REDIS_CELERY_DB, +} diff --git a/tests/utils_tests.py b/tests/utils_tests.py index 91d1ad39d5a55..035a7864feb20 100644 --- a/tests/utils_tests.py +++ b/tests/utils_tests.py @@ -27,7 +27,6 @@ import numpy from flask import Flask, g -from flask_caching import Cache import marshmallow from sqlalchemy.exc import ArgumentError @@ -37,7 +36,6 @@ from superset.models.core import Database, Log from superset.models.dashboard import Dashboard from superset.models.slice import Slice -from superset.utils.cache_manager import CacheManager from superset.utils.core import ( base_json_conv, cast_to_num, @@ -834,32 +832,6 @@ def test_parse_js_uri_path_items_item_optional(self): self.assertIsNone(parse_js_uri_path_item(None)) self.assertIsNotNone(parse_js_uri_path_item("item")) - def test_setup_cache_null_config(self): - app = Flask(__name__) - cache_config = {"CACHE_TYPE": "null"} - assert isinstance(CacheManager._setup_cache(app, cache_config), Cache) - - def test_setup_cache_standard_config(self): - app = Flask(__name__) - cache_config = { - "CACHE_TYPE": "redis", - "CACHE_DEFAULT_TIMEOUT": 60, - "CACHE_KEY_PREFIX": "superset_results", - "CACHE_REDIS_URL": "redis://localhost:6379/0", - } - assert isinstance(CacheManager._setup_cache(app, cache_config), Cache) is True - - def test_setup_cache_custom_function(self): - app = Flask(__name__) - CustomCache = type("CustomCache", (object,), {"__init__": lambda *args: None}) - - def init_cache(app): - return CustomCache(app, {}) - - assert ( - isinstance(CacheManager._setup_cache(app, init_cache), CustomCache) is True - ) - def test_get_stacktrace(self): with app.app_context(): app.config["SHOW_STACKTRACE"] = True