sadpandajoe commented on PR #45070: URL: https://github.com/apache/superset/pull/45070#issuecomment-6349122973
Thanks for jumping in on this one, and sorry for the long quiet stretch — appreciate you not just leaving it as a comment. Independently verified your fix (`7d7d54ec0c`) before replying: traced `get_cancel_query_id`'s callers directly, confirmed `_CancellationQuery` really does stub `query.id` as `None` for exactly this case, confirmed SQL Lab's real `Query` rows always have a persisted `id` by this call site so that path is unaffected, and confirmed `superset/mcp_service/worker.py`'s MCP-call cancellation goes through the exact same `capture_cancel_query_id`/stub, so it's covered too with no extra changes needed. Re-ran the full test suite (4629 passed), the real-ClickHouse-container test, and lint/type-check at your HEAD — all clean. This is the right fix. Nothing further needed from my side here — go ahead and treat this as resolved. On CodeAnt's other comment (the pre-dispatch-refresh-to-dispatch race): that one's an already-disclosed, intentionally-accepted limitation — it's called out explicitly in the PR description's "Known Limitations" section and in the same general TOCTOU family as the existing stop/cancel races this file already lives with for other engines. Leaving it as documented rather than chasing it further here. -- 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]
