sadpandajoe commented on code in PR #44406:
URL: https://github.com/apache/superset/pull/44406#discussion_r4322061779
##########
superset/common/query_context_processor.py:
##########
@@ -285,6 +279,18 @@ def get_df_payload_result(
# decision as the result cache, not the datasource-level default.
query_obj.force_query = force_query
query_obj.cache_timeout = timeout
+ # Resolved separately from the dataframe's force_query, against its own
+ # annotation_key. A forced GTF refresh's nonce marker is per (nonce,
+ # cache_key): the dataframe's marker is keyed on cache_key, so once
it's
+ # set, force_query goes False for every subsequent request carrying
that
+ # nonce, including a request for a *different* annotation access scope
+ # (its own annotation_key) that has never actually forced-refreshed its
+ # own entry. Reusing the dataframe's force_query there would read that
+ # scope's existing annotation cache entry instead of refreshing it.
+ annotation_force_query = (
+ self._resolve_forced_query(query_obj, annotation_key)
+ or timeout == CACHE_DISABLED_TIMEOUT
Review Comment:
The annotation-specific `timeout == CACHE_DISABLED_TIMEOUT` guard has no
test: the annotation payload tests all patch `get_cache_timeout` to
3600/123/300, so deleting this clause would leave them green and a chart with
caching disabled (`-1`) and a previously warmed annotation entry would serve
that stale entry instead of fetching live. Could a `get_df_payload_result` test
use `get_cache_timeout=-1` with `force=False` and a warm annotation entry, and
assert the live annotation fetch runs and its result replaces the stale value?
--
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]
