chore: Using cache factory method (#10887)

Co-authored-by: John Bodley <john.bodley@airbnb.com>

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.

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`:

superset/__init__.py

@@ -40,7 +40,7 @@ from superset.utils.log import DBEventLogger, get_event_logger_from_cfg_value
 #  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

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:

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 @@ FEATURE_FLAGS = {
     "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,
+}

tests/utils_tests.py

@@ -27,7 +27,6 @@ from unittest.mock import Mock, patch
 
 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.exceptions import CertificateException, SupersetException
 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 @@ class TestUtils(SupersetTestCase):
         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