rusackas commented on code in PR #43875:
URL: https://github.com/apache/superset/pull/43875#discussion_r3963444999
##########
superset/utils/cache.py:
##########
@@ -307,7 +307,7 @@ def wrapper(*args: Any, **kwargs: Any) -> Response: #
noqa: C901
wrapper.uncached = f # type: ignore
wrapper.cache_timeout = timeout # type: ignore
wrapper.make_cache_key = cache._memoize_make_cache_key( # type:
ignore # pylint: disable=protected-access
- make_name=None, timeout=timeout,
hash_method=configurable_hash_method
+ make_name=None, hash_method=configurable_hash_method
Review Comment:
Confirmed, and this looks fixed already — verified against the real
flask-caching 2.5.0 wheel.
Two commits landed after this comment:
- `64a57dc1c9` (fix(caching): remove obsolete timeout arg to
memoized_make_cache_key) stopped this `cached()` decorator from passing
`timeout=` into `cache._memoize_make_cache_key(...)`.
- `416a9f04d5` (fix(caching): remove obsolete timeout arg when pass to
module-level function memoized_make_cache_key) dropped
`timeout`/`forced_update` from `SupersetCache._memoize_make_cache_key`'s own
signature in `cache_manager.py`, and updated
`tests/unit_tests/utils/test_cache_manager.py` to match.
I downloaded the actual flask-caching==2.5.0 wheel and checked
`Cache._memoize_make_cache_key`'s real signature: `(self, make_name=None,
hash_method=None, source_check=None, args_to_ignore=None)` — no
`timeout`/`forced_update` anymore, which is exactly what the current override
now matches. I also reproduced the crash by re-running the pre-fix
(timeout-forwarding) version of this code against that same 2.5.0 install and
confirmed it raises `TypeError: Cache._memoize_make_cache_key() got an
unexpected keyword argument 'timeout'`, while the current code on this branch
does not. Leaving this thread open for @rusackas to close out.
##########
requirements/base.txt:
##########
@@ -105,7 +105,7 @@ et-xmlfile==2.0.0
# via openpyxl
filelock==3.20.3
# via -r requirements/base.in
-flask==2.3.3
+flask==3.1.3
Review Comment:
This is synced now — `requirements/base.txt` and
`requirements/development.txt` both pin `flask==3.1.3` on this line (via the
automated `10f99eed06` "build(deps): sync pinned requirements for Dependabot
pip PRs" commit), matching what CI actually installs and the Flask v2→v3 bump
called out in the PR title. Leaving this thread open for @rusackas to close out.
--
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]
