graceguo-supercat edited a comment on pull request #16367: URL: https://github.com/apache/superset/pull/16367#issuecomment-904015848
> Alternatively, we can show the Slack option only if `SLACK_API_TOKEN` is defined in the config. I actually think this is a good idea: - We should not show Slack if api token is None. Right now we show Slack as notification method even no API token set, and later user will get error message. This is not a correct behavior. - We could create `ALERT_REPORTS_NOTIFICATION_METHODS` internally (not in config.py) based on Slack settings, and pass it to front-end, it could avoid circular dependency. Otherwise, move those constants will touch about 20 files which makes me feel the changes are risky. -- 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]
