classification
Title: Change the decode_data default in smtpd to False
Type: enhancement Stage: resolved
Components: email, Library (Lib) Versions: Python 3.6
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: Arfrever, Duke.Dougal, Illirgway, barry, jesstess, lpolzer, maciej.szulik, python-dev, r.david.murray, richard, serhiy.storchaka, sreepriya, vstinner, zvyn
Priority: normal Keywords: patch

Created on 2016-05-15 19:20 by serhiy.storchaka, last changed 2016-06-23 14:47 by r.david.murray. This issue is now closed.

Files
File name Uploaded Description Edit
smtpd_decode_data_false.patch serhiy.storchaka, 2016-05-15 19:20 review
smtpd_decode_data_false_doc.patch serhiy.storchaka, 2016-05-16 18:42 review
smtpd_decode_data_false_doc2.patch serhiy.storchaka, 2016-05-29 20:04 review
smtpd_decode_data_false_doc3.patch serhiy.storchaka, 2016-05-29 20:12 review
Messages (16)
msg265645 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2016-05-15 19:20
Proposed patch changes the default value of the decode_data parameter for smtpd.SMTPChannel and smtpd.SMTPServer constructors. The default value True was deprecated since adding this parameter in issue19662.
msg265671 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2016-05-16 06:36
New changeset a31b9f353346 by Serhiy Storchaka in branch 'default':
Issue #27033: The default value of the decode_data parameter for
https://hg.python.org/cpython/rev/a31b9f353346
msg265694 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2016-05-16 14:17
Um, I changed to to commit review because I was planning to review it.  I don't expect to find any problems, but I'm surprised you committed it.

There does need to be a whatsnew entry, and specifically something in the porting section, since this is a behavior change.
msg265695 - (view) Author: Barry A. Warsaw (barry) * (Python committer) Date: 2016-05-16 14:26
aiosmtpd already defaults decode_data to False.  We should consider pulling this into 3.6 (not just for this reason of course).

http://aiosmtpd.readthedocs.io/en/latest/
msg265716 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2016-05-16 18:42
Oh, sorry, I understood this as a sign that you already made a review and don't have comments. I was surprised that you made this silently.

Here is a patch that adds an entry in the porting section of What's New. Please also check changes in the smtpd documentation. I reformulated some sentences.
msg265725 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2016-05-16 19:21
No problem.  I use 'commit review' as an indication that a committer should do a final review...before that, triage or general community people can be doing the reviews, then triage can move it to commit review to request the final review before commit.  What this means in practical terms is that if I do a search for 'email' issues in 'commit review' stage, I find all the issues I should be reviewing.

I'll try to do the review soon.
msg266625 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2016-05-29 20:04
Thank you for your review David. Updated patch addresses your doc comments.

As for bool calls, I think there are reasons to do this.
msg266626 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2016-05-29 20:12
Addressed other David's comment.
msg266633 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2016-05-29 20:33
What reasons?  It's not a big deal, but like I say it isn't our normal style.
msg266634 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2016-05-29 20:33
Oh, I forgot to say that the revisions look good to me.
msg266637 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2016-05-29 20:41
Since these arguments are saved for multiple testing, it is worth to ensure that every test returns consistent result. For public attribute there is additional benefit, it looks as boolean.
msg266638 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2016-05-29 20:44
New changeset 9abde519cc1e by Serhiy Storchaka in branch 'default':
Improved docs for issue27033. Based on comments by R. David Murray.
https://hg.python.org/cpython/rev/9abde519cc1e
msg266639 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2016-05-29 20:45
Python uses duck typing, though.  Maybe someone is depending on those public attributes not getting coerced to boolean.  Probably not, granted, since they are newish, but a python programmer might expect the value to get passed through.  I'm of two minds about the advisability of that, but given that someone *could* be depending on it, I think it would be better to not do the bool call, since there is no bug fixed by doing the bool call, making it an unnecessary change in behavior.
msg266640 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2016-05-29 20:52
New changeset 71813a05e488 by Serhiy Storchaka in branch 'default':
Issue #27033: Removed unnecessary the bool calls.
https://hg.python.org/cpython/rev/71813a05e488
msg269027 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2016-06-21 21:11
Can this issue be closed?
msg269115 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2016-06-23 14:47
Yes.  Thanks, Serhiy.
History
Date User Action Args
2016-06-23 14:47:06r.david.murraysetstatus: open -> closed
resolution: fixed
messages: + msg269115

stage: commit review -> resolved
2016-06-21 21:11:23serhiy.storchakasetmessages: + msg269027
2016-05-29 20:52:00python-devsetmessages: + msg266640
2016-05-29 20:45:48r.david.murraysetmessages: + msg266639
2016-05-29 20:44:03python-devsetmessages: + msg266638
2016-05-29 20:41:11serhiy.storchakasetmessages: + msg266637
2016-05-29 20:33:49r.david.murraysetmessages: + msg266634
2016-05-29 20:33:05r.david.murraysetmessages: + msg266633
2016-05-29 20:12:13serhiy.storchakasetfiles: + smtpd_decode_data_false_doc3.patch

messages: + msg266626
2016-05-29 20:04:11serhiy.storchakasetfiles: + smtpd_decode_data_false_doc2.patch

messages: + msg266625
2016-05-16 19:21:59r.david.murraysetmessages: + msg265725
2016-05-16 18:42:31serhiy.storchakasetfiles: + smtpd_decode_data_false_doc.patch

messages: + msg265716
2016-05-16 14:26:51barrysetmessages: + msg265695
2016-05-16 14:17:44r.david.murraysetstatus: closed -> open
resolution: fixed -> (no value)
messages: + msg265694

stage: resolved -> commit review
2016-05-16 06:38:11serhiy.storchakasetstatus: open -> closed
resolution: fixed
stage: commit review -> resolved
2016-05-16 06:36:52python-devsetmessages: + msg265671
2016-05-15 23:34:59r.david.murraysetcomponents: + email
2016-05-15 20:32:53r.david.murraysetstage: patch review -> commit review
2016-05-15 19:20:39serhiy.storchakacreate