Opened 4 months ago

Last modified 7 days ago

#35581 assigned Cleanup/optimization

Upgrade django.core.mail to use Python's modern email API

Reported by: Mike Edmunds Owned by: Mike Edmunds
Component: Core (Mail) Version: dev
Severity: Normal Keywords: email, compat32
Cc: bcail Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

django.core.mail.EmailMessage currently uses Python's "legacy" email API. Updating that code to Python's newer "modern" email API would fix bugs and substantially simplify it.

django-developers discussion

The update should be mostly transparent for users of Django's documented mail APIs, but will rework the internals of django.core.mail.message in ways that may require changes to some third-party email backends and other code that borrows undocumented (but popular) functions from that module. In addition, parts of Django's mail test suite will need reworking to avoid dependence on legacy implementation specifics.

Some relevant posts from django-developers:

Change History (11)

comment:1 by Tim Graham, 4 months ago

Triage Stage: UnreviewedAccepted
Version: dev

comment:2 by YashRaj1506, 3 months ago

Owner: changed from Mike Edmunds to YashRaj1506

I will try to solve this.

comment:3 by Mike Edmunds, 3 months ago

Owner: changed from YashRaj1506 to Mike Edmunds

@YashRaj1506 I have a bunch of work underway on this and will open a PR soon. Don't want you to needlessly duplicate effort. Code review on the PR would be very welcome once it's ready.

Part of the delay is I've run into a handful of open issues in Python's modern email API that may require further discussion:

Also trying to decide how best to handle Django's EmailMessage.encoding. It's not documented, but a bunch of tests cover setting it and checking the results. (The only relevant docs are this note about the DEFAULT_CHARSET setting also applying to django.core.mail.)

comment:4 by Mike Edmunds, 3 months ago

Has patch: set

I've opened two sequential PRs, as suggested in the django-developers discussion. Both are ready for review:

(The second PR is in my own Django fork so that it can be based on the first PR without overlap. I'll reopen it upstream once the first PR is closed. But reviews are welcome now if you don't mind working in my fork; just note any discussion history there won't transfer to the eventual upstream PR.)

Some additional notes are in the PR comments.

comment:5 by Mike Edmunds, 3 months ago

Patch needs improvement: set

comment:6 by Mike Edmunds, 2 months ago

Patch needs improvement: unset

comment:7 by bcail, 4 weeks ago

Cc: bcail added

comment:8 by Sarah Boyce <42296566+sarahboyce@…>, 7 days ago

In 889be2f:

Refs #35581 -- Clarified some test names and comments in mail tests.

comment:9 by Sarah Boyce <42296566+sarahboyce@…>, 7 days ago

In 4d76adfa:

Refs #35581 -- Verified attachments in the generated message in mail tests.

This also removed send() calls, as this doesn't check the serialized content, and
the backend tests cover sending.

comment:10 by Sarah Boyce <42296566+sarahboyce@…>, 7 days ago

In 00861c4c:

Refs #35581 -- Identified mail tests that check for Python 2 behavior.

This also removed a duplicate CTE case (that used to be distinct in Python 2).

comment:11 by Sarah Boyce <42296566+sarahboyce@…>, 7 days ago

In cf4d902:

Refs #35581 -- Reduced boilerplate in mail tests.

Note: See TracTickets for help on using tickets.
Back to Top