sadpandajoe commented on code in PR #39914:
URL: https://github.com/apache/superset/pull/39914#discussion_r3430301299
##########
superset/reports/notifications/slackv2.py:
##########
@@ -77,7 +77,18 @@ def _get_inline_files(
return ("pdf", [self._content.pdf])
return (None, [])
- @backoff.on_exception(backoff.expo, SlackApiError, factor=10, base=2,
max_tries=5)
+ # Retry on NotificationUnprocessableException (the wrapper that send()
+ # raises for transient Slack failures: SlackApiError, connection errors,
+ # and the SlackClientError catch-all). Retrying on SlackApiError directly
+ # would never fire because the try/except below converts it before the
+ # decorator can see it. Mirrors the pattern in webhook.py.
+ @backoff.on_exception(
+ backoff.expo,
+ NotificationUnprocessableException,
+ factor=10,
+ base=2,
+ max_tries=5,
+ )
Review Comment:
Confirmed — the retry behavior change is intentional and in scope for this
PR. The "not fixed here" note in the description predated the fix; I've updated
the description accordingly.
The decorator now retries on `NotificationUnprocessableException` (the type
`send()` actually raises for transient Slack failures) instead of
`SlackApiError`, which `send()` swallows before backoff can see it — mirroring
the existing `webhook.py` pattern. Transient failures retry up to 5× with
exponential backoff (~150s worst case, same budget as `webhook.py`);
non-transient errors
(`NotificationParamException`/`NotificationMalformedException`/`NotificationAuthorizationException`)
still surface immediately with no retry.
Tests assert both directions:
`test_v2_send_retries_then_succeeds_on_transient_failure` (fail twice then
succeed → 3 attempts), `test_v2_send_retries_on_transient_slack_api_error`
(persistent → 5 attempts), and `test_v2_send_does_not_retry_param_errors`
(permanent → 1 attempt).
--
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]
