[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content
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.

Sign up for GitHub

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

Jump to bottom

EMAIL: deprecation warning on mail servers with no authentication #19805

Open
2 tasks done
massamany opened this issue Nov 24, 2021 · 7 comments
Open
2 tasks done

EMAIL: deprecation warning on mail servers with no authentication #19805

massamany opened this issue Nov 24, 2021 · 7 comments
Labels
area:core good first issue kind:bug This is a clearly a bug

Comments

@massamany
Copy link

Apache Airflow version

2.2.0

Operating System

Ubuntu 18.04

Versions of Apache Airflow Providers

Not relevant here

Deployment

Other Docker-based deployment

Deployment details

No response

What happened

For my use cases, I need airflow to send emails, so I declared a Connection of "Email" type.
As the SMTP server do not require authentication, I left the user / password fields Empty in the Connection form. So that the user is an empty String in DB and the password is null.

When using the send_email method in email.py, it appears that I get the following deprecation warning in the logs : Fetching SMTP credentials from configuration variables will be deprecated in a future. release. Please set credentials using a connection instead.

In this case, this warning should not appear. In order to avoid it, I leave the user empty but declare a 1-space password, so that the followin condition in email.py is false and does not let the warning appear :

   if smtp_user is None or smtp_password is None:
        warnings.warn(
            "Fetching SMTP credentials from configuration variables will be deprecated in a future "
            "release. Please set credentials using a connection instead.",
            PendingDeprecationWarning,
            stacklevel=2,
        )

I think there are several solutions here, but I don't know which one will be prefered. But the one I would choose would be to ignore authentication in configuration as soon as there's a connection used or to warn as soon as the configuration file is used for the SMTP parameters.

What you expected to happen

No deprecation warning if a connection is used for STMP.

How to reproduce

  • declare a Email connexion without authentication
  • use it to send a mail
  • the undesired warning appears

Anything else

No response

Are you willing to submit PR?

  • Yes I am willing to submit a PR!

Code of Conduct
@massamany massamany added area:core kind:bug This is a clearly a bug labels Nov 24, 2021
@boring-cyborg
Copy link
boring-cyborg bot commented Nov 24, 2021

Thanks for opening your first issue here! Be sure to follow the issue template!

@potiuk
Copy link
Member
potiuk commented Nov 24, 2021

In #19781 We are discussing a possibility of moving the configuration of email fully to connection (now it is split between config and connection). I guess in the light of this discussion, the easiest way will be to use Email connection with empty user/password to indicate no authentication. Then in the future, the whole email configuration including host for example mightb be done via connection. I am not sure if that works now (can you please double check that @massamany).

Would that be a feasible solution for your case to create such a "no-user/no-password" connection?

@potiuk
Copy link
Member
potiuk commented Nov 24, 2021

BTW. Speaking of which - maybe you would like to take on the task of adding the capability of replacing the smtp configuration from the config with the one coming from connection @massamany ? That sounds not too difficult and could be a nice contribution and we can guide you there.

@massamany
Copy link
Author

@potiuk Yes you're right, I didn't see that at first glance. Only the user/pwd is retrieved from connection and the rest of SMTP connection info is retrieved from config file.
So it would be great to retrieve everything from the connection indeed.
But (there's always a "but"), here is the information needed for the SMTP configuration :

  • smtp_host : OK to retrieve from connection
  • smtp_port : OK to retrieve from connection
  • smtp_starttls : can't be retrieved from connection
  • smtp_ssl = can't be retrieved from connection
  • smtp_user = : OK to retrieve from connection
  • smtp_password = : OK to retrieve from connection
  • smtp_mail_from = can't be retrieved from connection
  • smtp_retry_limit = can't be retrieved from connection

So how to handle data that can't be retrieved from a connection ? Use the "extra" field ?
If not, the SMTP configuration would still be split between connection and configuration and it would be useless, especially if one wants for example to declare 2 SMTP connections, one with SSL and the other one without SSL.

In any case, I'm OK to take the PR. But you should know that I'm not an experienced Python dev (I come from the Java world). Still learning ;-) So I will certainly need help, especially for the testing part.

@potiuk
Copy link
Member
potiuk commented Nov 24, 2021

Yeah extra fields are the solution - and we can even customize Email connection to be able to edit those via Airflow UI.

No worries :) everyne must learn. But this one is rather easy - you can also take a look at the "First Time Airflow Contributor's Workshop" I run at OCSS in Mexico - to get some first steps into dev environment and "contribution vibe": https://youtu.be/kvccZizzfTk

@potiuk
Copy link
Member
potiuk commented Nov 24, 2021

BTW. I also came to Python from Java world and never looked back :)

@ITJamie
Copy link
ITJamie commented May 11, 2023

just a note on this. theres no logic right now to set the from_email with a connection. this needs to be updated to check for a from_email/smtp_mail_from variable in the email connection and prefer that over the from_email/smtp_mail_from configuration

@ITJamie ITJamie mentioned this issue May 11, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
area:core good first issue kind:bug This is a clearly a bug
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

move connectionless email depreciation warning ITJamie/airflow
3 participants
@potiuk @ITJamie @massamany