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

Change the decode_data default in smtpd to False #71220

Closed
serhiy-storchaka opened this issue May 15, 2016 · 16 comments
Closed

Change the decode_data default in smtpd to False #71220

serhiy-storchaka opened this issue May 15, 2016 · 16 comments
Labels
stdlib Python modules in the Lib dir topic-email type-feature A feature request or enhancement

Comments

@serhiy-storchaka
Copy link
Member

BPO 27033
Nosy @warsaw, @vstinner, @bitdancer, @serhiy-storchaka, @soltysh, @zvyn
Files
  • smtpd_decode_data_false.patch
  • smtpd_decode_data_false_doc.patch
  • smtpd_decode_data_false_doc2.patch
  • smtpd_decode_data_false_doc3.patch
    		•

    Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

    Show more details

    GitHub fields:

    assignee = None
closed_at = <Date 2016-06-23.14:47:06.282>
created_at = <Date 2016-05-15.19:20:39.421>
labels = ['type-feature', 'library', 'expert-email']
title = 'Change the decode_data default in smtpd to False'
updated_at = <Date 2016-06-23.14:47:06.280>
user = 'https://github.com/serhiy-storchaka'

    bugs.python.org fields:

    activity = <Date 2016-06-23.14:47:06.280>
actor = 'r.david.murray'
assignee = 'none'
closed = True
closed_date = <Date 2016-06-23.14:47:06.282>
closer = 'r.david.murray'
components = ['Library (Lib)', 'email']
creation = <Date 2016-05-15.19:20:39.421>
creator = 'serhiy.storchaka'
dependencies = []
files = ['42865', '42876', '43050', '43051']
hgrepos = []
issue_num = 27033
keywords = ['patch']
message_count = 16.0
messages = ['265645', '265671', '265694', '265695', '265716', '265725', '266625', '266626', '266633', '266634', '266637', '266638', '266639', '266640', '269027', '269115']
nosy_count = 14.0
nosy_names = ['barry', 'richard', 'vstinner', 'Arfrever', 'r.david.murray', 'jesstess', 'python-dev', 'serhiy.storchaka', 'maciej.szulik', 'lpolzer', 'Illirgway', 'Duke.Dougal', 'zvyn', 'sreepriya']
pr_nums = []
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'enhancement'
url = 'https://bugs.python.org/issue27033'
versions = ['Python 3.6']
    @serhiy-storchaka
    Copy link
    Member Author

    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 bpo-19662.

    @serhiy-storchaka serhiy-storchaka added stdlib Python modules in the Lib dir type-feature A feature request or enhancement labels May 15, 2016
    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented May 16, 2016

    New changeset a31b9f353346 by Serhiy Storchaka in branch 'default':
    Issue bpo-27033: The default value of the decode_data parameter for
    https://hg.python.org/cpython/rev/a31b9f353346

    @bitdancer
    Copy link
    Member

    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.

    @bitdancer bitdancer reopened this May 16, 2016
    @warsaw
    Copy link
    Member

    warsaw commented May 16, 2016

    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/

    @serhiy-storchaka
    Copy link
    Member Author

    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.

    @bitdancer
    Copy link
    Member

    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.

    @serhiy-storchaka
    Copy link
    Member Author

    Thank you for your review David. Updated patch addresses your doc comments.

    As for bool calls, I think there are reasons to do this.

    @serhiy-storchaka
    Copy link
    Member Author

    Addressed other David's comment.

    @bitdancer
    Copy link
    Member

    What reasons? It's not a big deal, but like I say it isn't our normal style.

    @bitdancer
    Copy link
    Member

    Oh, I forgot to say that the revisions look good to me.

    @serhiy-storchaka
    Copy link
    Member Author

    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.

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented May 29, 2016

    New changeset 9abde519cc1e by Serhiy Storchaka in branch 'default':
    Improved docs for bpo-27033. Based on comments by R. David Murray.
    https://hg.python.org/cpython/rev/9abde519cc1e

    @bitdancer
    Copy link
    Member

    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.

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented May 29, 2016

    New changeset 71813a05e488 by Serhiy Storchaka in branch 'default':
    Issue bpo-27033: Removed unnecessary the bool calls.
    https://hg.python.org/cpython/rev/71813a05e488

    @serhiy-storchaka
    Copy link
    Member Author

    Can this issue be closed?

    @bitdancer
    Copy link
    Member

    Yes. Thanks, Serhiy.

    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Assignees
    No one assigned
    Labels
    stdlib Python modules in the Lib dir topic-email type-feature A feature request or enhancement
    Projects
    None yet
    Milestone
    No milestone
    Development

    No branches or pull requests
    3 participants
    @warsaw @bitdancer @serhiy-storchaka