uranusjr commented on pull request #18200: URL: https://github.com/apache/airflow/pull/18200#issuecomment-919744257
I feel this PR is still valuable though, and the two PRs’ different implementations should be combined together into one cohesive solution. The previous PR (#18042) fills out the email field with the SMTP email config, which I’ve never been completely comfortable with, but the new email field introduced by this PR makes the Airflow config contain two distinct sets of email configurations, which would be very confusing to users. Perhaps the best way to combine the two would be to 1. Add the new `email_from` configs (but I don’t think it’s necessary to have separate fields `from_name` nad `from_email`; the email field can already contain a display name by default, e.g. `John Smith <[email protected]>`, so we only need one field (maybe with a better example and documentation so users know how to use it better). 2. Make `smtp_mail_from` optional and default to the `from_email` setting; the user can still set `smtp_mail_from` if needed to override this default only for SMTP. 3. Use `email_from` in the SES implementation. This is only my initial thought and other ideas are definitely welcomed. @ecerulm and @ignaski you two can also discuss which one of you should take the lead on this one and /or how to split work between. -- 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]
