codeant-ai-for-open-source[bot] commented on code in PR #45561:
URL: https://github.com/apache/superset/pull/45561#discussion_r4336380946
##########
superset/sql/execution/executor.py:
##########
@@ -680,16 +683,24 @@ def execute(
QueryStatus.TIMED_OUT,
"Query exceeded the timeout limit",
start_time,
+ query=query,
)
except SupersetSecurityException as ex:
- return self._create_error_result(QueryStatus.FAILED, str(ex),
start_time)
- except (OAuth2RedirectError, OAuth2Error):
+ return self._create_error_result(
+ QueryStatus.FAILED, str(ex), start_time, query=query
+ )
+ except (OAuth2RedirectError, OAuth2Error) as ex:
# Let OAuth2 exceptions propagate so callers (MCP, API) can
# handle them with context-appropriate responses.
+ self._create_error_result(
+ QueryStatus.FAILED, str(ex), start_time, query=query
+ )
raise
Review Comment:
✅ **CodeAnt verified this suggestion was addressed in subsequent commits and
marked this thread resolved** as of `65d3fe4`.
`_create_error_result` now catches persistence failures, logs them, and
suppresses rollback errors, so metadata recording failures do not prevent the
original OAuth exception from being re-raised.
<sub>If that's not right, unresolve this thread and CodeAnt will leave it
open.</sub>
<!-- codeant-auto-resolve-reply -->
##########
superset/sql/execution/executor.py:
##########
@@ -1008,14 +1020,24 @@ def _create_error_result(
:param start_time: Start time for calculating execution duration
:param partial_results: Optional list of StatementResult from
successful
statements before the failure
+ :param query: Persisted audit record, if execution reached that stage
:returns: QueryResult with error status
"""
from superset_core.queries.types import QueryResult as QueryResultType
+ if query is not None:
+ # A failed metadata flush must be rolled back before recording
failure.
+ db.session.rollback() # pylint: disable=consider-using-transaction
+ query.status = status.value
Review Comment:
✅ **CodeAnt verified this suggestion was addressed in subsequent commits and
marked this thread resolved** as of `65d3fe4`.
Error persistence now uses `commit_query_unless_stopped`; when that guard
detects a stopped query, the result status is set to `STOPPED` instead of
overwriting it with the execution failure status.
<sub>If that's not right, unresolve this thread and CodeAnt will leave it
open.</sub>
<!-- codeant-auto-resolve-reply -->
--
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]
