rusackas commented on code in PR #43149:
URL: https://github.com/apache/superset/pull/43149#discussion_r3877996002
##########
superset/utils/cache.py:
##########
@@ -162,9 +162,12 @@ def wrap(f: Callable[..., Any]) -> Callable[..., Any]:
def wrapped_f(*args: Any, **kwargs: Any) -> Any:
should_cache = kwargs.pop("cache", True)
force = kwargs.pop("force", False)
- cache_timeout = kwargs.pop(
- "cache_timeout", app.config["CACHE_DEFAULT_TIMEOUT"]
- )
+ # callers may explicitly pass ``cache_timeout=None`` (eg, when a
database
+ # has no custom metadata cache timeout configured), which should
fall back
+ # to the default timeout rather than be forwarded to the cache
backend.
+ cache_timeout = kwargs.pop("cache_timeout", None)
+ if cache_timeout is None:
+ cache_timeout = app.config["CACHE_DEFAULT_TIMEOUT"]
Review Comment:
This looks stale against the current diff, the `cache_timeout is None`
fallback is already placed below the `not should_cache` early return, so the
`cache=False` bypass never touches `app.config`.
`test_memoized_func_skip_cache_pops_cache_timeout` pins exactly that and
asserts `mock_config.__getitem__.assert_not_called()`.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
