You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by "armandmasseaugit (via GitHub)" <gi...@apache.org> on 2025/07/29 20:45:27 UTC
[PR] Improve send_mime_email: better SMTP connection handling and docstring clarification [airflow]
armandmasseaugit opened a new pull request, #53899:
URL: https://github.com/apache/airflow/pull/53899
### Overview
This PR refactors the `send_mime_email` function in `airflow/utils/email.py` to improve SMTP connection handling by leveraging Airflow's connection system more cleanly. It also updates the function’s docstring for better clarity.
### Changes
- Improved support for extracting SMTP parameters (host, port, TLS, login, password) from Airflow connections.
- Ensured fallback to `conf.get` remains consistent.
- Updated docstring to document function behavior and parameters properly.
### Motivation
This change improves the reliability and configurability of email sending in Airflow, particularly in environments that rely on Airflow-managed connections for SMTP configuration. It also reduces confusion for developers by clarifying the function's behavior.
### Related issues
None
---
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Improve send_mime_email: better SMTP connection handling and docstring clarification [airflow]
Posted by "armandmasseaugit (via GitHub)" <gi...@apache.org>.
armandmasseaugit commented on PR #53899:
URL: https://github.com/apache/airflow/pull/53899#issuecomment-3346331749
Hey, could I get a review please ? Thanks a lot ! I think that the reviews requested as codeowners are due to the simlinks changes that the precommit hook did.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Improve send_mime_email: better SMTP connection handling and docstring clarification [airflow]
Posted by "ashb (via GitHub)" <gi...@apache.org>.
ashb commented on code in PR #53899:
URL: https://github.com/apache/airflow/pull/53899#discussion_r2391827080
##########
airflow-core/newsfragments/53899.feature.rst:
##########
@@ -0,0 +1 @@
+SMTP email configuration now supports both legacy config and Airflow Connections for more flexibility and security. You can use either the classic [smtp] section in airflow.cfg or a Connection (type SMTP) for sending emails. This allows seamless migration and more dynamic setups.
Review Comment:
Agreed.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Improve send_mime_email: better SMTP connection handling and docstring clarification [airflow]
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on PR #53899:
URL: https://github.com/apache/airflow/pull/53899#issuecomment-3239346498
Also tests should be fixed of course.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Improve send_mime_email: better SMTP connection handling and docstring clarification [airflow]
Posted by "ashb (via GitHub)" <gi...@apache.org>.
ashb closed pull request #53899: Improve send_mime_email: better SMTP connection handling and docstring clarification
URL: https://github.com/apache/airflow/pull/53899
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Improve send_mime_email: better SMTP connection handling and docstring clarification [airflow]
Posted by "armandmasseaugit (via GitHub)" <gi...@apache.org>.
armandmasseaugit commented on PR #53899:
URL: https://github.com/apache/airflow/pull/53899#issuecomment-3227047985
Done. Thank you
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Improve send_mime_email: better SMTP connection handling and docstring clarification [airflow]
Posted by "armandmasseaugit (via GitHub)" <gi...@apache.org>.
armandmasseaugit commented on PR #53899:
URL: https://github.com/apache/airflow/pull/53899#issuecomment-3143575494
Thank you for your guidance! I will update my PR with these improvements.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Improve send_mime_email: better SMTP connection handling and docstring clarification [airflow]
Posted by "armandmasseaugit (via GitHub)" <gi...@apache.org>.
armandmasseaugit commented on PR #53899:
URL: https://github.com/apache/airflow/pull/53899#issuecomment-3241580805
Done. Thanks for the review. The simlinks were updated automatically when I ran the precommit hook. Moreover, I don't understand why the CI failed, I think it will be resolved when rebasing with main branch because the failing tests are not related to what I added.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Improve send_mime_email: better SMTP connection handling and docstring clarification [airflow]
Posted by "armandmasseaugit (via GitHub)" <gi...@apache.org>.
armandmasseaugit commented on PR #53899:
URL: https://github.com/apache/airflow/pull/53899#issuecomment-3346334159
@hussein-awala @ashb @kaxil @amoghrajesh
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Improve send_mime_email: better SMTP connection handling and docstring clarification [airflow]
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on PR #53899:
URL: https://github.com/apache/airflow/pull/53899#issuecomment-3225772946
Some conflicts and failing tests to solve - rebase is needed
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Improve send_mime_email: better SMTP connection handling and docstring clarification [airflow]
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on PR #53899:
URL: https://github.com/apache/airflow/pull/53899#issuecomment-3215740991
Will do next week
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Improve send_mime_email: better SMTP connection handling and docstring clarification [airflow]
Posted by "boring-cyborg[bot] (via GitHub)" <gi...@apache.org>.
boring-cyborg[bot] commented on PR #53899:
URL: https://github.com/apache/airflow/pull/53899#issuecomment-3134019800
Congratulations on your first Pull Request and welcome to the Apache Airflow community! If you have any issues or are unsure about any anything please check our Contributors' Guide (https://github.com/apache/airflow/blob/main/contributing-docs/README.rst)
Here are some useful points:
- Pay attention to the quality of your code (ruff, mypy and type annotations). Our [pre-commits]( https://github.com/apache/airflow/blob/main/contributing-docs/08_static_code_checks.rst#prerequisites-for-pre-commit-hooks) will help you with that.
- In case of a new feature add useful documentation (in docstrings or in `docs/` directory). Adding a new operator? Check this short [guide](https://github.com/apache/airflow/blob/main/airflow-core/docs/howto/custom-operator.rst) Consider adding an example DAG that shows how users should use it.
- Consider using [Breeze environment](https://github.com/apache/airflow/blob/main/dev/breeze/doc/README.rst) for testing locally, it's a heavy docker but it ships with a working Airflow and a lot of integrations.
- Be patient and persistent. It might take some time to get a review or get the final approval from Committers.
- Please follow [ASF Code of Conduct](https://www.apache.org/foundation/policies/conduct) for all communication including (but not limited to) comments on Pull Requests, Mailing list and Slack.
- Be sure to read the [Airflow Coding style]( https://github.com/apache/airflow/blob/main/contributing-docs/05_pull_requests.rst#coding-style-and-best-practices).
- Always keep your Pull Requests rebased, otherwise your build might fail due to changes not related to your commits.
Apache Airflow is a community-driven project and together we are making it better 🚀.
In case of doubts contact the developers at:
Mailing List: dev@airflow.apache.org
Slack: https://s.apache.org/airflow-slack
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Improve send_mime_email: better SMTP connection handling and docstring clarification [airflow]
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on PR #53899:
URL: https://github.com/apache/airflow/pull/53899#issuecomment-3141375890
While it is good idea in general to make smtp handling more consistent, This change is almost certainly missing a number of things and should be much more "complete" - as it is all about a 'core" functionality.
* documentation explaining all the various cases how email can be configured - including some examples of configuring it via connection and configuration
* fallback for host/port - seems that for example SMTP mandatory host value is always overwritten by the host value from connection
* the type specification in doctsring is "extra" - the type is read from type hints
* tests shoudl be added to tests various combinations of the parameter
* back-compatibility consideration - likely some notes and newsfragment should be added to explain what the users are supposed to do.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Improve send_mime_email: better SMTP connection handling and docstring clarification [airflow]
Posted by "armandmasseaugit (via GitHub)" <gi...@apache.org>.
armandmasseaugit commented on PR #53899:
URL: https://github.com/apache/airflow/pull/53899#issuecomment-3161495569
Hello, just a gentle ping 😊 — let me know if anything is missing or could be improved in this PR. Thanks!
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Improve send_mime_email: better SMTP connection handling and docstring clarification [airflow]
Posted by "eladkal (via GitHub)" <gi...@apache.org>.
eladkal commented on code in PR #53899:
URL: https://github.com/apache/airflow/pull/53899#discussion_r2387647540
##########
airflow-core/newsfragments/53899.feature.rst:
##########
@@ -0,0 +1 @@
+SMTP email configuration now supports both legacy config and Airflow Connections for more flexibility and security. You can use either the classic [smtp] section in airflow.cfg or a Connection (type SMTP) for sending emails. This allows seamless migration and more dynamic setups.
Review Comment:
I am -0.5 of making core coupled more with email/smtp.
From my perspective all core email related stuff should be deprecated and users should migrate entirely to SMTP provider.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Improve send_mime_email: better SMTP connection handling and docstring clarification [airflow]
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on code in PR #53899:
URL: https://github.com/apache/airflow/pull/53899#discussion_r2311988025
##########
task-sdk/src/airflow/sdk/_shared/secrets_masker:
##########
@@ -1 +1 @@
-../../../../../shared/secrets_masker/src/airflow_shared/secrets_masker
\ No newline at end of file
+../../../../../shared/secrets_masker/src/airflow_shared/secrets_masker
Review Comment:
Those changes are not related
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Improve send_mime_email: better SMTP connection handling and docstring clarification [airflow]
Posted by "armandmasseaugit (via GitHub)" <gi...@apache.org>.
armandmasseaugit commented on PR #53899:
URL: https://github.com/apache/airflow/pull/53899#issuecomment-3199410106
Friendly ping @potiuk - Would you have time to take another look at this PR? All previous comments have been addressed. Thank you!
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org