bito-code-review[bot] commented on PR #42484:
URL: https://github.com/apache/superset/pull/42484#issuecomment-5147980119
<!-- Bito Reply -->
The concern regarding the blanket downgrade of `SlackApiError` logging to
`WARNING` is valid. Currently, the change in `superset/utils/slack.py` applies
this downgrade to all `SlackApiError` exceptions, which includes critical
infrastructure failures like rate limits or server-side errors that should
ideally remain at `ERROR` level for observability.
To address this, you could implement a check within the `except
SlackApiError` block to distinguish between expected authentication errors
(e.g., `not_authed`, `invalid_auth`) and other API errors. You could then log
authentication-related errors at `WARNING` and re-raise or log other
`SlackApiError` instances at `ERROR` level.
**superset/utils/slack.py**
```
except SlackApiError as ex:
if ex.response.get("error") in ("not_authed", "invalid_auth"):
logger.warning("Auth error fetching Slack channels: %s", str(ex))
else:
logger.error("Slack API error: %s", str(ex), exc_info=True)
raise
```
--
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]
