rusackas commented on code in PR #44552:
URL: https://github.com/apache/superset/pull/44552#discussion_r4118876363
##########
superset/common/query_context_processor.py:
##########
@@ -305,7 +306,15 @@ def get_df_payload_result(
)
)
- query_result = self.get_query_result(query_obj)
+ # Make the warehouse query cancellable by its owner for the
span
+ # of its execution, so Explore's Stop button kills the query
+ # rather than just abandoning the response. No-op for engines
+ # without cancel support, and for requests carrying no
client_id.
+ with cancellable_chart_query(
+ self._query_context.client_id,
+ getattr(self._qc_datasource, "database", None),
+ ):
+ query_result = self.get_query_result(query_obj)
annotation_data = self.get_annotation_data(query_obj)
Review Comment:
This one's real, and I don't think it's a safe one-line fix.
get_viz_annotation_data can run a genuine second warehouse query, against a
*different* chart's own datasource than the one cancellable_chart_query was
opened for. The context manager's capture sink closes over a single fixed
target database (the outer chart's), so naively pulling get_annotation_data
into the same `with` block would misattribute any captured cancel id to the
wrong database whenever the annotation layer points at a chart on a different
one, cancelling nothing, or worse, the wrong session.
Same category as the Trino gap already called out in the PR description,
real, not something to guess a fix for here. Adding it to that section so it's
not just sitting in a resolved thread. Follow-up would need
cancellable_chart_query to accept (or the sink to resolve) a database per
capture rather than one fixed at open time.
--
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]
