sadpandajoe commented on code in PR #44406:
URL: https://github.com/apache/superset/pull/44406#discussion_r4321202387
##########
tests/unit_tests/tasks/test_query_cancel.py:
##########
@@ -159,6 +159,26 @@ def
test_capture_registers_abort_handler_once_when_id_available() -> None:
assert ordered.index("set_cancellation") < ordered.index("on_abort")
+def test_capture_ignores_cursor_from_a_different_database() -> None:
Review Comment:
`sink(MagicMock(), None)` and this direct foreign-database call never go
through `Database.get_raw_connection -> notify_cursor(cursor, self)`, so a
regression that rejects every non-None database, or drops the `self` forwarding
in `models/core.py`, would leave all of these tests green while cancellation
stops working for real chart queries. Could a test drive `notify_cursor(cursor,
database)` with the task's own database and assert the handler registers once,
and with another database and assert it doesn't?
##########
superset/common/query_context_processor.py:
##########
@@ -358,6 +362,39 @@ def get_df_payload_result(
# nonce reads the freshly-cached result instead of recomputing
it.
self._mark_force_executed(query_obj, cache_key,
cache.result_persisted)
+ # Annotation data is fetched per requesting user (and, for chart-backed
+ # layers, scoped by the referenced chart datasource's RLS), so it is
+ # resolved and cached under its own entry — independent of whether the
+ # (shareable) dataframe above was a hit or a miss — rather than forcing
+ # every viewer of the same chart onto their own full dataframe copy.
+ annotation_data: dict[str, Any] = {}
+ if (
+ query_obj
+ and query_obj.annotation_layers
+ and cache.status != QueryStatus.FAILED
+ ):
+ annotation_start_ns = time.perf_counter_ns()
+ try:
+ annotation_data = self._get_annotation_data_cached(
Review Comment:
With the dataframe key no longer scoped by viewer, a second viewer can join
the first viewer's SHARED chart-data task, but that task still runs the
annotation fetch under the submitter's access. If the submitter's annotation
fetch raises `SemanticResultCompletenessError`, or fails validation inside a
contribution totals prerequisite (`is_totals_prerequisite` re-raises a FAILED
payload in `execute_chart_query`), the task fails for the joined viewer too.
For example, viewer A lacks `can_read Annotation` while viewer B has it and a
warm annotation entry; both force-refresh the same expired chart, B joins A's
task, and B gets a task failure even though B's own execution would have
succeeded. Should annotation failures be kept out of the dataframe-keyed task
outcome, or the task identity stay split by annotation scope?
--
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]
