Re: [PR] feat(deps): bump `flask-caching` from 2.4.1 to 2.5.0 + bump `flask` from v2 to v3 [superset]

["rusackas (via GitHub)" <gi...@apache.org>]

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: 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