[GitHub] [superset] john-bodley commented on a change in pull request #18976: chore(cache): default to SimpleCache in debug mode

[GitBox <gi...@apache.org>]

john-bodley commented on a change in pull request #18976:
URL: https://github.com/apache/superset/pull/18976#discussion_r816975361



##########
File path: superset/config.py
##########
@@ -576,29 +577,43 @@ 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 timeout, applies to all cache backends unless specifically overridden in
+
+# Default cache config, applies to all cache backends unless specifically overridden in
 # each cache config.
-CACHE_DEFAULT_TIMEOUT = int(timedelta(days=1).total_seconds())
+def DEFAULT_CACHE_CONFIG_FUNC(  # pylint: disable=invalid-name
+    app: Flask,
+) -> CacheConfig:
+    default_timeout = app.config.get("CACHE_DEFAULT_TIMEOUT")
+    if default_timeout is None:
+        default_timeout = int(timedelta(days=1).total_seconds())
+    else:
+        logger.warning(
+            "The config flag CACHE_DEFAULT_TIMEOUT has been deprecated "

Review comment:
       This isn't quite true, i.e., this flag is defined by Flask-Caching per [here](https://flask-caching.readthedocs.io/en/latest/#configuring-flask-caching) but I get the gist of the comment.

##########
File path: superset/utils/cache.py
##########
@@ -55,7 +60,11 @@ def set_and_log_cache(
     if isinstance(cache_instance.cache, NullCache):
         return
 
-    timeout = cache_timeout if cache_timeout else config["CACHE_DEFAULT_TIMEOUT"]
+    timeout = (
+        cache_timeout
+        if cache_timeout

Review comment:
       ```suggestion
           if cache_timeout is not None
   ```
   
   I know this is just a rewrite of the above but `cache_timeout` could be `0` i.e., per Flask-Caching, 
   
   > A timeout of 0 indicates that the cache never expires.
   
   so a truthiness check isn't suffice.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org