I3eka commented on code in PR #45729:
URL: https://github.com/apache/superset/pull/45729#discussion_r4402750775
##########
superset/mcp_service/worker.py:
##########
@@ -791,7 +793,10 @@ def capture(self) -> None:
def register(self) -> None:
"""Publish a usable handle, including one obtained after the
deadline."""
with self.call.lock:
- if self.spec.has_implicit_cancel():
+ if self.spec.has_implicit_cancel() or (
+ self.spec.supports_cancel_on_live_cursor is True
+ and callable(getattr(self.cursor, "cancel", None))
+ ):
Review Comment:
Fixed in a29f77c730819674ae56a73f1d780ea7d00144c5. I applied the four
suggestions together: the worker asks the engine spec whether the current
driver can safely cancel its live cursor, and SQL Server requires `pyodbc` plus
a callable `cancel()`. The existing cursor lifetime lock and
implicit-cancellation path are unchanged. Synapse and Sybase still opt out
through the base capability check.
Confirmed the `pytds` regression fails before the fix and passes after it. I
also covered rejection of a non-pyodbc cursor that exposes `cancel()`, and
checked the subclass opt-outs through the new hook.
Merged Apache master `c7f17e0bb40f7a0283bce3a57f69e3ec91256834` and retained
both sets of tests when resolving the MSSQL test-file conflict. All 113 tests
in the MSSQL, SQL-execution, MCP worker and migration-head suites pass; all
changed-file pre-commit checks (including mypy, Ruff and pylint) pass. No live
warehouse cancellation or deployment was performed.
--
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]
