EnxDev commented on code in PR #44336:
URL: https://github.com/apache/superset/pull/44336#discussion_r4060970872
##########
superset/commands/report/execute.py:
##########
@@ -1687,7 +1877,7 @@ def _handle_retry_or_error(
# send the retry-failure notification *after* the attempt ran so
# the email reflects what happened, not what is about to be
# scheduled. ("You will receive an update after each retry.")
- if current_attempt > 0:
+ if 0 < current_attempt < max_attempts:
Review Comment:
Could we preserve the last per-attempt notice when `send_failed_reports` is
off? With `retry_max_attempts=1`, `retry_notify_recipients=True`, and
`send_failed_reports=False` (the default), this skips the only retry notice.
The final-failure notice is also disabled, and `send_error()` only emails
editors, so configured recipients get no failure notification despite opting in.
I reproduced this through `ReportNotTriggeredErrorState.next()` for alerts
and reports. The report case notifies the recipient with the base branch's
retry handler, but only the editor with this handler. Could we keep one
terminal failure notice for the opted-in recipients when no final-failure
notice replaces it? A one-retry test with both values of `send_failed_reports`
would cover the gap while retaining the duplicate-notice protection.
--
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]
