codeant-ai-for-open-source[bot] commented on code in PR #43149:
URL: https://github.com/apache/superset/pull/43149#discussion_r3781272082
##########
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:
**Suggestion:** Normalize `cache_timeout` only after checking
`should_cache`. When callers pass `cache=False`, this new unconditional access
to `app.config` makes the bypass path require an active Flask application
context and a `CACHE_DEFAULT_TIMEOUT` entry, even though caching was explicitly
disabled; it can raise a “Working outside of application context” error before
the wrapped function runs. Move the fallback lookup below the early `not
should_cache` return. [api mismatch]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ❌ Uncached `memoized_func` calls can fail before execution.
- ⚠️ Bypass behavior depends unnecessarily on Flask configuration.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=f7065a238a0e43e5a1afb5a9d39ffceb&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
[](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=f7065a238a0e43e5a1afb5a9d39ffceb&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset/utils/cache.py
**Line:** 165:170
**Comment:**
*Api Mismatch: Normalize `cache_timeout` only after checking
`should_cache`. When callers pass `cache=False`, this new unconditional access
to `app.config` makes the bypass path require an active Flask application
context and a `CACHE_DEFAULT_TIMEOUT` entry, even though caching was explicitly
disabled; it can raise a “Working outside of application context” error before
the wrapped function runs. Move the fallback lookup below the early `not
should_cache` return.
Validate the correctness of the flagged issue. If correct, How can I resolve
this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask
user if the user wants to fix the rest of the comments as well. if said yes,
then fetch all the comments validate the correctness and implement a minimal fix
```
</details>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F43149&comment_hash=65d28650080dcbbb21aa82d69fe94991f9176bb0c6c1d59096c036d12abf43c9&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F43149&comment_hash=65d28650080dcbbb21aa82d69fe94991f9176bb0c6c1d59096c036d12abf43c9&reaction=dislike'>👎</a>
--
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]
