-
Notifications
You must be signed in to change notification settings - Fork 23.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
mail: fixed STARTTLS module working with python 3.7.0 #47412
Conversation
Hi @k0ste, thank you for submitting this pull-request! |
HI, |
@rangapv what is your environment? What exactly error you have? You are apply this patch? |
(cherry picked from commit 8c9070e)
smtp = smtplib.SMTP_SSL(host=host, timeout=timeout) | ||
code, smtpmessage = smtp.connect(host, port=port) | ||
smtp = smtplib.SMTP_SSL(host=host, port=port, timeout=timeout) | ||
code, smtpmessage = smtp.connect(host, port) |
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 is strange to me: the documentation of smtplib
says that if you pass host/port to the constructor, it'll call connect
for you:
If the optional host and port parameters are given, the SMTP connect() method is called with those parameters during initialization.
So you're connecting to the SMTP server twice -- why?
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.
First connect is a secure layer.
(cherry picked from commit 8c9070e)
smtp = smtplib.SMTP_SSL(host=host, timeout=timeout) | ||
code, smtpmessage = smtp.connect(host, port=port) | ||
smtp = smtplib.SMTP_SSL(host=host, port=port, timeout=timeout) | ||
code, smtpmessage = smtp.connect(host, port) |
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.
I tested the mail module on Ansible v2.7.1 on Python 3.7.1 and if you specify secure: never
the mail module works fine. Changes are only necessary to be applied to the attempt to use ssl (which you did on lines 267 & 268). By changing the regular smtp section, it breaks functionality for ansible users that run on python 2.7.x. Is there a reason you changed this section as well?
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.
@tpeyton, because smtp_ssl
and smtp_over_tls
is different things.
|
I confirm #49016 fixes mail.py for me on Python 2.7.15. |
SUMMARY
#44552 was fixed module working with
SSL
.This PR also fixed module working with
STARTTLS
.ISSUE TYPE
COMPONENT NAME
mail
ANSIBLE VERSION
ADDITIONAL INFORMATION
After backport to 2.7 should resolve #46673.