sadpandajoe commented on code in PR #35800:
URL: https://github.com/apache/superset/pull/35800#discussion_r2453032123
##########
superset/commands/report/execute.py:
##########
@@ -752,14 +767,21 @@ def next(self) -> None:
return
self.send()
self.update_report_schedule_and_log(ReportState.SUCCESS)
+ except ReportScheduleUnexpectedError:
+ # Don't try to log again if logging itself failed
+ raise
except (SupersetErrorsException, Exception) as first_ex:
error_message = str(first_ex)
if isinstance(first_ex, SupersetErrorsException):
error_message = ";".join([error.message for error in
first_ex.errors])
- self.update_report_schedule_and_log(
- ReportState.ERROR, error_message=error_message
- )
+ try:
Review Comment:
From codex:
> When update_report_schedule_and_log raises ReportScheduleUnexpectedError,
the bare raise in this except ReportScheduleUnexpectedError clause re-raises
the logging failure and short-circuits the outer except
(SupersetErrorsException, Exception) as first_ex:. The original first_ex (e.g.,
a real send error) never escapes, even though the comment says we want to
surface it. This regresses error observability—callers only see “Report
schedule was modified…” instead of the actual failure. Please re-raise first_ex
(or otherwise ensure the original exception propagates) while still logging the
StaleDataError.
--
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]
