(superset) branch master updated: fix(cache): fall back to default timeout when cache_timeout is None (#43149)

[ru...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

rusackas pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/superset.git


The following commit(s) were added to refs/heads/master by this push:
     new 3ddc3b1d569 fix(cache): fall back to default timeout when cache_timeout is None (#43149)
3ddc3b1d569 is described below

commit 3ddc3b1d5695b7409269d93e844d614fe20381d3
Author: Chandan P <95...@users.noreply.github.com>
AuthorDate: Fri Aug 28 10:20:53 2026 +0530

    fix(cache): fall back to default timeout when cache_timeout is None (#43149)
---
 superset/utils/cache.py              | 14 ++++--
 tests/unit_tests/utils/cache_test.py | 93 ++++++++++++++++++++++++++++++++++++
 2 files changed, 104 insertions(+), 3 deletions(-)

diff --git a/superset/utils/cache.py b/superset/utils/cache.py
index 161b40832d2..f636a576c1e 100644
--- a/superset/utils/cache.py
+++ b/superset/utils/cache.py
@@ -162,13 +162,21 @@ def memoized_func(key: str, cache: Cache = cache_manager.cache) -> Callable[...,
         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"]
-            )
+            # always popped, even when caching is skipped, so it is never forwarded
+            # to the decorated function as an unexpected keyword argument.
+            cache_timeout = kwargs.pop("cache_timeout", None)
 
             if not should_cache:
                 return f(*args, **kwargs)
 
+            # 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.
+            # the config lookup happens here so the uncached path stays independent
+            # of the Flask app config.
+            if cache_timeout is None:
+                cache_timeout = app.config["CACHE_DEFAULT_TIMEOUT"]
+
             # format the key using args/kwargs passed to the decorated function
             signature = inspect.signature(f)
             bound_args = signature.bind(*args, **kwargs)
diff --git a/tests/unit_tests/utils/cache_test.py b/tests/unit_tests/utils/cache_test.py
index 2d0d48e5983..b1555c10cea 100644
--- a/tests/unit_tests/utils/cache_test.py
+++ b/tests/unit_tests/utils/cache_test.py
@@ -54,6 +54,99 @@ def test_memoized_func(mocker: MockerFixture) -> None:
     assert result == 43
 
 
+def test_memoized_func_none_cache_timeout(mocker: MockerFixture) -> None:
+    """
+    An explicit ``cache_timeout=None`` falls back to ``CACHE_DEFAULT_TIMEOUT``.
+
+    Databases without a custom metadata cache timeout pass ``None`` explicitly, and
+    forwarding it to the cache backend breaks backends that require an integer.
+    """
+    from superset.utils.cache import memoized_func
+
+    _patch_config(mocker)
+    cache = mocker.MagicMock()
+    cache.get.return_value = None
+
+    decorator = memoized_func("db:{self.id}:schema:{schema}:table_list", cache)
+    decorated = decorator(lambda self, schema: 42)
+
+    self = mocker.MagicMock()
+    self.id = 1
+
+    result = decorated(self, "public", cache_timeout=None)
+    assert result == 42
+    cache.set.assert_called_once_with("db:1:schema:public:table_list", 42, timeout=100)
+
+
+def test_memoized_func_custom_cache_timeout(mocker: MockerFixture) -> None:
+    """
+    An explicit ``cache_timeout`` takes precedence over ``CACHE_DEFAULT_TIMEOUT``.
+    """
+    from superset.utils.cache import memoized_func
+
+    _patch_config(mocker)
+    cache = mocker.MagicMock()
+    cache.get.return_value = None
+
+    decorator = memoized_func("db:{self.id}:schema:{schema}:table_list", cache)
+    decorated = decorator(lambda self, schema: 42)
+
+    self = mocker.MagicMock()
+    self.id = 1
+
+    result = decorated(self, "public", cache_timeout=42)
+    assert result == 42
+    cache.set.assert_called_once_with("db:1:schema:public:table_list", 42, timeout=42)
+
+
+def test_memoized_func_disabled_cache_timeout(mocker: MockerFixture) -> None:
+    """
+    A timeout of -1 (``CACHE_DISABLED_TIMEOUT``) skips the cache set.
+    """
+    from superset.utils.cache import memoized_func
+
+    _patch_config(mocker)
+    cache = mocker.MagicMock()
+    cache.get.return_value = None
+
+    decorator = memoized_func("db:{self.id}:schema:{schema}:table_list", cache)
+    decorated = decorator(lambda self, schema: 42)
+
+    self = mocker.MagicMock()
+    self.id = 1
+
+    result = decorated(self, "public", cache_timeout=-1)
+    assert result == 42
+    cache.set.assert_not_called()
+
+
+def test_memoized_func_skip_cache_pops_cache_timeout(mocker: MockerFixture) -> None:
+    """
+    ``cache=False`` skips caching without touching the config or the wrapped function.
+
+    ``cache_timeout`` must still be popped so it is not forwarded to the decorated
+    function, which does not accept it. Callers such as
+    ``get_all_table_names_in_schema`` pass ``cache`` and ``cache_timeout`` together.
+    """
+    from superset.utils.cache import memoized_func
+
+    mock_config = mocker.patch("superset.utils.cache.app.config", MagicMock())
+    cache = mocker.MagicMock()
+
+    decorator = memoized_func("db:{self.id}:schema:{schema}:table_list", cache)
+    decorated = decorator(lambda self, schema: 42)
+
+    self = mocker.MagicMock()
+    self.id = 1
+
+    result = decorated(self, "public", cache=False, cache_timeout=None)
+
+    assert result == 42
+    cache.get.assert_not_called()
+    cache.set.assert_not_called()
+    mock_config.__getitem__.assert_not_called()
+
+
 def _make_cache_instance(mocker: MockerFixture) -> MagicMock:
     """A cache instance whose ``.cache`` is not a ``NullCache``."""
     cache_instance = mocker.MagicMock()