From d5f8d42007763a953cd83dae2439eba4d258f1d8 Mon Sep 17 00:00:00 2001 From: Ville Brofeldt Date: Wed, 2 Mar 2022 15:39:16 +0200 Subject: [PATCH] remove default cache config --- UPDATING.md | 2 +- docs/docs/installation/cache.mdx | 23 ++--- superset/common/query_context_processor.py | 2 +- superset/config.py | 27 +++--- superset/sql_lab.py | 4 +- superset/utils/cache.py | 7 +- superset/utils/cache_manager.py | 93 ++++++++++--------- superset/viz.py | 2 +- tests/integration_tests/cache_tests.py | 13 +-- .../dashboards/filter_state/api_tests.py | 1 - .../explore/form_data/api_tests.py | 1 - .../integration_tests/superset_test_config.py | 19 ++-- .../superset_test_config_thumbnails.py | 2 + tests/integration_tests/viz_tests.py | 5 +- 14 files changed, 99 insertions(+), 102 deletions(-) diff --git a/UPDATING.md b/UPDATING.md index 77a9f7da9b754..c3d1de14d56e6 100644 --- a/UPDATING.md +++ b/UPDATING.md @@ -26,7 +26,7 @@ assists people when migrating to a new version. ### Breaking Changes -- [18976](https://github.com/apache/superset/pull/18976): A new `DEFAULT_CACHE_CONFIG` parameter has been introduced in `config.py` which makes it possible to define a default cache config that will be used as the basis for all cache configs. When running the app in debug mode, the app will default to use `SimpleCache`; in other cases the default cache type will be `NullCache`. In addition, `DEFAULT_CACHE_TIMEOUT` has been deprecated and moved into `DEFAULT_CACHE_CONFIG` (will be removed in Superset 2.0). For installations using Redis or other caching backends, it is recommended to set the default cache options in `DEFAULT_CACHE_CONFIG` to ensure the primary cache is always used if new caches are added. +- [18976](https://github.com/apache/superset/pull/18976): When running the app in debug mode, the app will default to use `SimpleCache` for `FILTER_STATE_CACHE_CONFIG` and `EXPLORE_FORM_DATA_CACHE_CONFIG`. When running in non-debug mode, a cache backend will need to be defined, otherwise the application will fail to start. For installations using Redis or other caching backends, it is recommended to use the same backend for both cache configs. - [17881](https://github.com/apache/superset/pull/17881): Previously simple adhoc filter values on string columns were stripped of enclosing single and double quotes. To fully support literal quotes in filters, both single and double quotes will no longer be removed from filter values. - [17984](https://github.com/apache/superset/pull/17984): Default Flask SECRET_KEY has changed for security reasons. You should always override with your own secret. Set `PREVIOUS_SECRET_KEY` (ex: PREVIOUS_SECRET_KEY = "\2\1thisismyscretkey\1\2\\e\\y\\y\\h") with your previous key and use `superset re-encrypt-secrets` to rotate you current secrets - [15254](https://github.com/apache/superset/pull/15254): Previously `QUERY_COST_FORMATTERS_BY_ENGINE`, `SQL_VALIDATORS_BY_ENGINE` and `SCHEDULED_QUERIES` were expected to be defined in the feature flag dictionary in the `config.py` file. These should now be defined as a top-level config, with the feature flag dictionary being reserved for boolean only values. diff --git a/docs/docs/installation/cache.mdx b/docs/docs/installation/cache.mdx index c91cd51176004..e86382b3c16c4 100644 --- a/docs/docs/installation/cache.mdx +++ b/docs/docs/installation/cache.mdx @@ -7,25 +7,21 @@ version: 1 ## Caching -Superset uses [Flask-Caching](https://flask-caching.readthedocs.io/) for caching purpose. Default caching options -can be set by overriding the `DEFAULT_CACHE_CONFIG` in your `superset_config.py`. Unless overridden, the default -cache type will be set to `SimpleCache` when running in debug mode, and `NullCache` otherwise. - -Currently there are five separate cache configurations to provide additional security and more granular customization options: +Superset uses [Flask-Caching](https://flask-caching.readthedocs.io/) for caching purpose. Configuring caching is as easy as providing a custom cache config in your +`superset_config.py` that complies with [the Flask-Caching specifications](https://flask-caching.readthedocs.io/en/latest/#configuring-flask-caching). +Flask-Caching supports various caching backends, including Redis, Memcached, SimpleCache (in-memory), or the +local filesystem. Custom cache backends are also supported. See [here](https://flask-caching.readthedocs.io/en/latest/#custom-cache-backends) for specifics. +The following cache configurations can be customized: - Metadata cache (optional): `CACHE_CONFIG` - Charting data queried from datasets (optional): `DATA_CACHE_CONFIG` - SQL Lab query results (optional): `RESULTS_BACKEND`. See [Async Queries via Celery](/docs/installation/async-queries-celery) for details - Dashboard filter state (required): `FILTER_STATE_CACHE_CONFIG`. - Explore chart form data (required): `EXPLORE_FORM_DATA_CACHE_CONFIG` -Configuring caching is as easy as providing a custom cache config in your -`superset_config.py` that complies with [the Flask-Caching specifications](https://flask-caching.readthedocs.io/en/latest/#configuring-flask-caching). -Flask-Caching supports various caching backends, including Redis, Memcached, SimpleCache (in-memory), or the -local filesystem. Custom cache backends are also supported. See [here](https://flask-caching.readthedocs.io/en/latest/#custom-cache-backends) for specifics. - -Note that Dashboard and Explore caching is required, and configuring the application with either of these caches set to `NullCache` will -cause the application to fail on startup. Also keep in mind, tht when running Superset on a multi-worker setup, a dedicated cache is required. -For this we recommend running either Redis or Memcached: +Please note, that Dashboard and Explore caching is required. When running Superset in debug mode, both Explore and Dashboard caches will default to `SimpleCache`; +However, trying to run Superset in non-debug mode without defining a cache for these will cause the application to fail on startup. When running +superset in single-worker mode, any cache backend is supported. However, when running Superset in on a multi-worker setup, a dedicated cache is required. For this +we recommend using either Redis or Memcached: - Redis (recommended): we recommend the [redis](https://pypi.python.org/pypi/redis) Python package - Memcached: we recommend using [pylibmc](https://pypi.org/project/pylibmc/) client library as @@ -37,6 +33,7 @@ For chart data, Superset goes up a “timeout search path”, from a slice's con to the datasource’s, the database’s, then ultimately falls back to the global default defined in `DATA_CACHE_CONFIG`. +## Celery beat 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/common/query_context_processor.py b/superset/common/query_context_processor.py index f247512d47628..a313d9d8258d6 100644 --- a/superset/common/query_context_processor.py +++ b/superset/common/query_context_processor.py @@ -385,7 +385,7 @@ def get_cache_timeout(self) -> int: cache_timeout_rv = self._query_context.get_cache_timeout() if cache_timeout_rv: return cache_timeout_rv - return app.config["DEFAULT_CACHE_CONFIG"]["CACHE_DEFAULT_TIMEOUT"] + return config["CACHE_DEFAULT_TIMEOUT"] def cache_key(self, **extra: Any) -> str: """ diff --git a/superset/config.py b/superset/config.py index 59eb069e5dd51..86d7b45c4711a 100644 --- a/superset/config.py +++ b/superset/config.py @@ -36,7 +36,7 @@ from cachelib.base import BaseCache from celery.schedules import crontab from dateutil import tz -from flask import Blueprint, Flask +from flask import Blueprint from flask_appbuilder.security.manager import AUTH_DB from pandas._libs.parsers import STR_NA_VALUES # pylint: disable=no-name-in-module from typing_extensions import Literal @@ -543,8 +543,8 @@ def _try_json_readsha(filepath: str, length: int) -> Optional[str]: # Also used by Alerts & Reports # --------------------------------------------------- THUMBNAIL_SELENIUM_USER = "admin" -# thumbnail cache (will be merged with DEFAULT_CACHE_CONFIG) THUMBNAIL_CACHE_CONFIG: CacheConfig = { + "CACHE_TYPE": "NullCache", "CACHE_NO_NULL_WARNING": True, } @@ -576,27 +576,26 @@ def _try_json_readsha(filepath: str, length: int) -> Optional[str]: # Setup image size default is (300, 200, True) # IMG_SIZE = (300, 200, True) -# Default cache for Superset objects (will be used as the base for all cache configs) -DEFAULT_CACHE_CONFIG: CacheConfig = { - "CACHE_TYPE": "NullCache", - "CACHE_DEFAULT_TIMEOUT": int(timedelta(days=1).total_seconds()), -} +# Default cache timeout, applies to all cache backends unless specifically overridden in +# each cache config. +CACHE_DEFAULT_TIMEOUT = int(timedelta(days=1).total_seconds()) -# Default cache for Superset objects (will be merged with DEFAULT_CACHE_CONFIG) -CACHE_CONFIG: CacheConfig = {} +# Default cache for Superset objects +CACHE_CONFIG: CacheConfig = {"CACHE_TYPE": "NullCache"} -# Cache for datasource metadata and query results (will be merged with -# DEFAULT_CACHE_CONFIG) -DATA_CACHE_CONFIG: CacheConfig = {} +# Cache for datasource metadata and query results +DATA_CACHE_CONFIG: CacheConfig = {"CACHE_TYPE": "NullCache"} -# Cache for filters state (will be merged with DEFAULT_CACHE_CONFIG) +# Cache for dashboard filter state (`CACHE_TYPE` defaults to `SimpleCache` when +# running in debug mode unless overridden) FILTER_STATE_CACHE_CONFIG: CacheConfig = { "CACHE_DEFAULT_TIMEOUT": int(timedelta(days=90).total_seconds()), # should the timeout be reset when retrieving a cached value "REFRESH_TIMEOUT_ON_RETRIEVAL": True, } -# Cache for chart form data (will be merged with DEFAULT_CACHE_CONFIG) +# Cache for explore form data state (`CACHE_TYPE` defaults to `SimpleCache` when +# running in debug mode unless overridden) EXPLORE_FORM_DATA_CACHE_CONFIG: CacheConfig = { "CACHE_DEFAULT_TIMEOUT": int(timedelta(days=7).total_seconds()), # should the timeout be reset when retrieving a cached value diff --git a/superset/sql_lab.py b/superset/sql_lab.py index 90349aa58bf9d..8fac419cf0ba6 100644 --- a/superset/sql_lab.py +++ b/superset/sql_lab.py @@ -538,9 +538,7 @@ def execute_sql_statements( # pylint: disable=too-many-arguments, too-many-loca ) cache_timeout = database.cache_timeout if cache_timeout is None: - cache_timeout = app.config["DEFAULT_CACHE_CONFIG"][ - "CACHE_DEFAULT_TIMEOUT" - ] + cache_timeout = config["CACHE_DEFAULT_TIMEOUT"] compressed = zlib_compress(serialized_payload) logger.debug( diff --git a/superset/utils/cache.py b/superset/utils/cache.py index 1adb8af7b9f17..c10f296e1cac4 100644 --- a/superset/utils/cache.py +++ b/superset/utils/cache.py @@ -21,7 +21,7 @@ from functools import wraps from typing import Any, Callable, Dict, Optional, TYPE_CHECKING, Union -from flask import current_app as app, Flask, request +from flask import current_app as app, request from flask_caching import Cache from flask_caching.backends import NullCache from werkzeug.wrappers.etag import ETagResponseMixin @@ -29,7 +29,6 @@ from superset import db from superset.extensions import cache_manager from superset.models.cache import CacheKey -from superset.typing import CacheConfig from superset.utils.core import json_int_dttm_ser from superset.utils.hashing import md5_sha_from_dict @@ -59,7 +58,7 @@ def set_and_log_cache( timeout = ( cache_timeout if cache_timeout is not None - else app.config["DEFAULT_CACHE_CONFIG"]["CACHE_DEFAULT_TIMEOUT"] + else app.config["CACHE_DEFAULT_TIMEOUT"] ) try: dttm = datetime.utcnow().isoformat().split(".")[0] @@ -151,7 +150,7 @@ def etag_cache( """ if max_age is None: - max_age = app.config["DEFAULT_CACHE_CONFIG"]["CACHE_DEFAULT_TIMEOUT"] + max_age = app.config["CACHE_DEFAULT_TIMEOUT"] def decorator(f: Callable[..., Any]) -> Callable[..., Any]: @wraps(f) diff --git a/superset/utils/cache_manager.py b/superset/utils/cache_manager.py index f40e6018950e9..a0c759035fc2b 100644 --- a/superset/utils/cache_manager.py +++ b/superset/utils/cache_manager.py @@ -21,8 +21,6 @@ from flask_babel import gettext as _ from flask_caching import Cache -from superset.typing import CacheConfig - logger = logging.getLogger(__name__) @@ -30,62 +28,71 @@ class CacheManager: def __init__(self) -> None: super().__init__() - self._default_cache_config: CacheConfig = {} - self.cache = Cache() - self.data_cache = Cache() - self.thumbnail_cache = Cache() - self.filter_state_cache = Cache() - self.explore_form_data_cache = Cache() + self._cache = Cache() + self._data_cache = Cache() + self._thumbnail_cache = Cache() + self._filter_state_cache = Cache() + self._explore_form_data_cache = Cache() + @staticmethod def _init_cache( - self, app: Flask, cache: Cache, cache_config_key: str, required: bool = False + app: Flask, cache: Cache, cache_config_key: str, required: bool = False ) -> None: - config = {**self._default_cache_config, **app.config[cache_config_key]} - if required and config["CACHE_TYPE"] in ("null", "NullCache"): + cache_config = app.config[cache_config_key] + cache_type = cache_config.get("CACHE_TYPE") + if app.debug and cache_type is None: + cache_threshold = cache_config.get("CACHE_THRESHOLD", math.inf) + cache_config.update( + {"CACHE_TYPE": "SimpleCache", "CACHE_THRESHOLD": cache_threshold,} + ) + + if "CACHE_DEFAULT_TIMEOUT" not in cache_config: + default_timeout = app.config.get("CACHE_DEFAULT_TIMEOUT") + cache_config["CACHE_DEFAULT_TIMEOUT"] = default_timeout + + if required and cache_type in ("null", "NullCache"): raise Exception( _( "The CACHE_TYPE `%(cache_type)s` for `%(cache_config_key)s` is not " - "supported. It is recommended to use `RedisCache`, `MemcachedCache` " - "or another dedicated caching backend for production deployments", - cache_type=config["CACHE_TYPE"], + "supported. It is recommended to use `RedisCache`, " + "`MemcachedCache` or another dedicated caching backend for " + "production deployments", + cache_type=cache_config["CACHE_TYPE"], cache_config_key=cache_config_key, ), ) - cache.init_app(app, config) + cache.init_app(app, cache_config) def init_app(self, app: Flask) -> None: - if app.debug: - self._default_cache_config = { - "CACHE_TYPE": "SimpleCache", - "CACHE_THRESHOLD": math.inf, - } - else: - self._default_cache_config = {} - - default_timeout = app.config.get("CACHE_DEFAULT_TIMEOUT") - if default_timeout is not None: - self._default_cache_config["CACHE_DEFAULT_TIMEOUT"] = default_timeout - logger.warning( - _( - "The global config flag `CACHE_DEFAULT_TIMEOUT` has been " - "deprecated and will be removed in Superset 2.0. Please set " - "default cache options in the `DEFAULT_CACHE_CONFIG` parameter" - ), - ) - self._default_cache_config = { - **self._default_cache_config, - **app.config["DEFAULT_CACHE_CONFIG"], - } - - self._init_cache(app, self.cache, "CACHE_CONFIG") - self._init_cache(app, self.data_cache, "DATA_CACHE_CONFIG") - self._init_cache(app, self.thumbnail_cache, "THUMBNAIL_CACHE_CONFIG") + self._init_cache(app, self._cache, "CACHE_CONFIG") + self._init_cache(app, self._data_cache, "DATA_CACHE_CONFIG") + self._init_cache(app, self._thumbnail_cache, "THUMBNAIL_CACHE_CONFIG") self._init_cache( - app, self.filter_state_cache, "FILTER_STATE_CACHE_CONFIG", required=True + app, self._filter_state_cache, "FILTER_STATE_CACHE_CONFIG", required=True ) self._init_cache( app, - self.explore_form_data_cache, + self._explore_form_data_cache, "EXPLORE_FORM_DATA_CACHE_CONFIG", required=True, ) + + @property + def data_cache(self) -> Cache: + return self._data_cache + + @property + def cache(self) -> Cache: + return self._cache + + @property + def thumbnail_cache(self) -> Cache: + return self._thumbnail_cache + + @property + def filter_state_cache(self) -> Cache: + return self._filter_state_cache + + @property + def explore_form_data_cache(self) -> Cache: + return self._explore_form_data_cache diff --git a/superset/viz.py b/superset/viz.py index f1ce2c5543b21..3a519bb1f9e83 100644 --- a/superset/viz.py +++ b/superset/viz.py @@ -429,7 +429,7 @@ def cache_timeout(self) -> int: return self.datasource.database.cache_timeout if config["DATA_CACHE_CONFIG"].get("CACHE_DEFAULT_TIMEOUT") is not None: return config["DATA_CACHE_CONFIG"]["CACHE_DEFAULT_TIMEOUT"] - return app.config["DEFAULT_CACHE_CONFIG"]["CACHE_DEFAULT_TIMEOUT"] + return config["CACHE_DEFAULT_TIMEOUT"] def get_json(self) -> str: return json.dumps( diff --git a/tests/integration_tests/cache_tests.py b/tests/integration_tests/cache_tests.py index ddb9d91c6d424..a7da8a50d2a59 100644 --- a/tests/integration_tests/cache_tests.py +++ b/tests/integration_tests/cache_tests.py @@ -64,17 +64,12 @@ def test_no_data_cache(self): @pytest.mark.usefixtures("load_birth_names_dashboard_with_slices") def test_slice_data_cache(self): # Override cache config - default_cache_config = app.config["DEFAULT_CACHE_CONFIG"] - - app.config["DEFAULT_CACHE_CONFIG"] = { - **default_cache_config, - "CACHE_DEFAULT_TIMEOUT": 100, - } data_cache_config = app.config["DATA_CACHE_CONFIG"] - + cache_default_timeout = app.config["CACHE_DEFAULT_TIMEOUT"] + app.config["CACHE_DEFAULT_TIMEOUT"] = 100 app.config["DATA_CACHE_CONFIG"] = { + "CACHE_TYPE": "SimpleCache", "CACHE_DEFAULT_TIMEOUT": 10, - "CACHE_KEY_PREFIX": "superset_data_cache", } cache_manager.init_app(app) @@ -105,5 +100,5 @@ def test_slice_data_cache(self): # reset cache config app.config["DATA_CACHE_CONFIG"] = data_cache_config - app.config["DEFAULT_CACHE_CONFIG"] = default_cache_config + app.config["CACHE_DEFAULT_TIMEOUT"] = cache_default_timeout cache_manager.init_app(app) diff --git a/tests/integration_tests/dashboards/filter_state/api_tests.py b/tests/integration_tests/dashboards/filter_state/api_tests.py index 2a11980535efd..3816f6ac87337 100644 --- a/tests/integration_tests/dashboards/filter_state/api_tests.py +++ b/tests/integration_tests/dashboards/filter_state/api_tests.py @@ -62,7 +62,6 @@ def admin_id() -> int: @pytest.fixture(autouse=True) def cache(dashboard_id, admin_id): - cache_manager.init_app(app) entry: Entry = {"owner": admin_id, "value": value} cache_manager.filter_state_cache.set(cache_key(dashboard_id, key), entry) diff --git a/tests/integration_tests/explore/form_data/api_tests.py b/tests/integration_tests/explore/form_data/api_tests.py index 112eec2bb839b..4b646a03586de 100644 --- a/tests/integration_tests/explore/form_data/api_tests.py +++ b/tests/integration_tests/explore/form_data/api_tests.py @@ -74,7 +74,6 @@ def dataset_id() -> int: @pytest.fixture(autouse=True) def cache(chart_id, admin_id, dataset_id): - cache_manager.init_app(app) entry: TemporaryExploreState = { "owner": admin_id, "dataset_id": dataset_id, diff --git a/tests/integration_tests/superset_test_config.py b/tests/integration_tests/superset_test_config.py index 3e3b6151f8986..7c862328294b5 100644 --- a/tests/integration_tests/superset_test_config.py +++ b/tests/integration_tests/superset_test_config.py @@ -87,13 +87,6 @@ def GET_FEATURE_FLAGS_FUNC(ff): REDIS_CACHE_DB = os.environ.get("REDIS_CACHE_DB", 4) -DEFAULT_CACHE_CONFIG = { - "CACHE_TYPE": "SimpleCache", - "CACHE_THRESHOLD": math.inf, - "CACHE_DEFAULT_TIMEOUT": int(timedelta(minutes=10).total_seconds()), -} - - CACHE_CONFIG = { "CACHE_TYPE": "RedisCache", "CACHE_DEFAULT_TIMEOUT": int(timedelta(minutes=1).total_seconds()), @@ -107,6 +100,18 @@ def GET_FEATURE_FLAGS_FUNC(ff): "CACHE_KEY_PREFIX": "superset_data_cache", } +FILTER_STATE_CACHE_CONFIG = { + "CACHE_TYPE": "SimpleCache", + "CACHE_THRESHOLD": math.inf, + "CACHE_DEFAULT_TIMEOUT": int(timedelta(minutes=10).total_seconds()), +} + +EXPLORE_FORM_DATA_CACHE_CONFIG = { + "CACHE_TYPE": "SimpleCache", + "CACHE_THRESHOLD": math.inf, + "CACHE_DEFAULT_TIMEOUT": int(timedelta(minutes=10).total_seconds()), +} + GLOBAL_ASYNC_QUERIES_JWT_SECRET = "test-secret-change-me-test-secret-change-me" ALERT_REPORTS_WORKING_TIME_OUT_KILL = True diff --git a/tests/integration_tests/superset_test_config_thumbnails.py b/tests/integration_tests/superset_test_config_thumbnails.py index 2e375931c3867..9f621efabbf4d 100644 --- a/tests/integration_tests/superset_test_config_thumbnails.py +++ b/tests/integration_tests/superset_test_config_thumbnails.py @@ -53,6 +53,8 @@ def GET_FEATURE_FLAGS_FUNC(ff): AUTH_ROLE_PUBLIC = "Public" EMAIL_NOTIFICATIONS = False +CACHE_CONFIG = {"CACHE_TYPE": "SimpleCache"} + REDIS_HOST = os.environ.get("REDIS_HOST", "localhost") REDIS_PORT = os.environ.get("REDIS_PORT", "6379") REDIS_CELERY_DB = os.environ.get("REDIS_CELERY_DB", 2) diff --git a/tests/integration_tests/viz_tests.py b/tests/integration_tests/viz_tests.py index 56af2240f480d..465fdb26ef581 100644 --- a/tests/integration_tests/viz_tests.py +++ b/tests/integration_tests/viz_tests.py @@ -173,10 +173,7 @@ def test_cache_timeout(self): app.config["DATA_CACHE_CONFIG"]["CACHE_DEFAULT_TIMEOUT"] = None datasource.database.cache_timeout = None test_viz = viz.BaseViz(datasource, form_data={}) - self.assertEqual( - app.config["DEFAULT_CACHE_CONFIG"]["CACHE_DEFAULT_TIMEOUT"], - test_viz.cache_timeout, - ) + self.assertEqual(app.config["CACHE_DEFAULT_TIMEOUT"], test_viz.cache_timeout) # restore DATA_CACHE_CONFIG timeout app.config["DATA_CACHE_CONFIG"]["CACHE_DEFAULT_TIMEOUT"] = data_cache_timeout