-
Notifications
You must be signed in to change notification settings - Fork 14.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[WIP] Migrate core EmailOperator
to SMTP provider
#30531
base: main
Are you sure you want to change the base?
[WIP] Migrate core EmailOperator
to SMTP provider
#30531
Conversation
895f99d
to
2f93d11
Compare
This pull request has been automatically marked as stale because it has not had recent activity. It will be closed in 5 days if no further activity occurs. Thank you for your contributions. |
|
||
# for credentials, we use the connection if it exists, otherwise we use the config | ||
if connection.login is None or connection.password is None: | ||
warnings.warn( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This warning assumes incorrectly that it has to fallback to cfg settings to find a username or password for smtp. Not all smtp servers require authentication to send mail.
It can not be assumed that if a connection has no username or password that the connection config is wrong and thus falling back to legacy config
This pull request has been automatically marked as stale because it has not had recent activity. It will be closed in 5 days if no further activity occurs. Thank you for your contributions. |
@hussein-awala i think now that main branch is Airflow 3 this PR should be easier |
@hussein-awala will you have time to complete this one? |
I will try to complete it by tomorrow. |
@eladkal I think there is no need to migrate it, we can delete the resources we want to deprecate and clean the code. I created this draft: #46041. For version 2.11, and since the provider operator and SMTP Notifer have been introduced for quite some time, we don't need to create a complicated backport, IMHO it will be enough to inform/warn users about the upcoming deprecation. WDYT? |
I agree. The goal is to remove email integration from core completely |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need to keep it?
We migrated almost all core operators to standard provider.
Should we just remove this file?
closes: #30530
In this PR, we migrate core
EmailOperator
to SMTP provider, and we deprecate the core one.