Re: [Django] #35864: Warn that EmailMessage.connection option is ignored when using send_messages()

18 views
Skip to first unread message

Django

unread,
Oct 28, 2024, 4:02:38 PM10/28/24
to django-...@googlegroups.com
#35864: Warn that EmailMessage.connection option is ignored when using
send_messages()
-------------------------------------+-------------------------------------
Reporter: Mike Edmunds | Owner: Mike
Type: | Edmunds
Cleanup/optimization | Status: assigned
Component: Core (Mail) | Version: 5.1
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by Mike Edmunds):

* owner: (none) => Mike Edmunds
* status: new => assigned


Old description:

> The EmailMessage [https://docs.djangoproject.com/en/5.1/topics/email
> /#emailmessage-
> objects:~:text=sending%20the%20email.-,connection,-%3A%20An%20email%20backend
> connection] option is ignored when using send_messages() to
> [https://docs.djangoproject.com/en/5.1/topics/email/#topics-sending-
> multiple-emails send multiple messages] as suggested in the docs.
>
> Here's some code that ''won't'' in fact use special handling for EU users
> as the author expects:
>
> {{{#!python
> from django.core import mail
>
> def get_notification_emails():
> notification_emails = []
> for user in get_users_needing_notification():
> email = mail.EmailMultiAlternatives(to=[user.email], ...)
> # Override the ESP for users in the EU; otherwise the default is
> fine.
> if user.profile.must_use_eu_data_processor():
> email.connection = mail.get_connection(hostname="smtp-
> eu.example.net")
> notification_emails.append(email)
> return notification_emails
>
> def send_periodic_notification_emails():
> messages = get_notification_emails()
> # We replaced this loop with send_messages() to optimize sending.
> # https://docs.djangoproject.com/en/5.1/topics/email/#sending-
> multiple-emails
> # for message in messages:
> # message.send()
> connection = mail.get_connection()
> connection.send_messages(messages)
> }}}
>
> (You ''probably'' wouldn't make this mistake writing that code all at the
> same time, but you might stumble into it if those functions were in
> different files and the send_messages() optimization got added later.)
>
> If we were designing the EmailMessage API from scratch, a clearer design
> would probably handle `connection` as an EmailMessage.send() parameter,
> rather than an EmailMessage property. I don't think it's worth the effort
> to try to change that now. (But there's an opportunity to avoid similar
> confusion when we introduce `provider` as the successor to `connection`
> in #35514.)
>
> Suggested fix: in the EmailMessage.connection docs, note that: "This
> option is ignored when using `send_messages()`." And maybe in the
> send_messages() section note that it "overrides any `connection` option
> on individual messages."
>
> We could also log a warning in smtp.EmailBackend.send_messages(), if any
> of the messages has a connection that is not self. (But each EmailBackend
> implements its own send_messages(), so this wouldn't help with third
> party backends.)

New description:

The EmailMessage [https://docs.djangoproject.com/en/5.1/topics/email
/#emailmessage-
objects:~:text=sending%20the%20email.-,connection,-%3A%20An%20email%20backend
connection] option is ignored when using send_messages() to
[https://docs.djangoproject.com/en/5.1/topics/email/#topics-sending-
multiple-emails send multiple messages] as suggested in the docs.

Here's some code that ''won't'' in fact use special handling for EU users
as the author expects:

{{{#!python
from django.core import mail

def get_notification_emails():
notification_emails = []
for user in get_users_needing_notification():
email = mail.EmailMultiAlternatives(to=[user.email], ...)
# Override the ESP for users in the EU; otherwise the default is
fine.
if user.profile.must_use_eu_data_processor():
email.connection = mail.get_connection(host="smtp-
eu.example.net")
notification_emails.append(email)
return notification_emails

def send_periodic_notification_emails():
messages = get_notification_emails()
# We replaced this loop with send_messages() to optimize sending.
# https://docs.djangoproject.com/en/5.1/topics/email/#sending-
multiple-emails
# for message in messages:
# message.send()
connection = mail.get_connection()
connection.send_messages(messages)
}}}

(You ''probably'' wouldn't make this mistake writing that code all at the
same time, but you might stumble into it if those functions were in
different files and the send_messages() optimization got added later.)

If we were designing the EmailMessage API from scratch, a clearer design
would probably handle `connection` as an EmailMessage.send() parameter,
rather than an EmailMessage property. I don't think it's worth the effort
to try to change that now. (But there's an opportunity to avoid similar
confusion when we introduce `provider` as the successor to `connection` in
#35514.)

Suggested fix: in the EmailMessage.connection docs, note that: "This
option is ignored when using `send_messages()`." And maybe in the
send_messages() section note that it "overrides any `connection` option on
individual messages."

We could also log a warning in smtp.EmailBackend.send_messages(), if any
of the messages has a connection that is not self. (But each EmailBackend
implements its own send_messages(), so this wouldn't help with third party
backends.)

--
--
Ticket URL: <https://code.djangoproject.com/ticket/35864#comment:2>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

Django

unread,
Oct 28, 2024, 4:03:25 PM10/28/24
to django-...@googlegroups.com
#35864: Warn that EmailMessage.connection option is ignored when using
send_messages()
-------------------------------------+-------------------------------------
Reporter: Mike Edmunds | Owner: Mike
Type: | Edmunds
Cleanup/optimization | Status: assigned
Component: Core (Mail) | Version: 5.1
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by Mike Edmunds):

* has_patch: 0 => 1

--
Ticket URL: <https://code.djangoproject.com/ticket/35864#comment:3>

Django

unread,
Oct 29, 2024, 8:35:06 AM10/29/24
to django-...@googlegroups.com
#35864: Warn that EmailMessage.connection option is ignored when using
send_messages()
-------------------------------------+-------------------------------------
Reporter: Mike Edmunds | Owner: Mike
Type: | Edmunds
Cleanup/optimization | Status: assigned
Component: Core (Mail) | Version: 5.1
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by Sarah Boyce):

* needs_better_patch: 0 => 1

--
Ticket URL: <https://code.djangoproject.com/ticket/35864#comment:4>

Django

unread,
Oct 29, 2024, 1:31:59 PM10/29/24
to django-...@googlegroups.com
#35864: Warn that EmailMessage.connection option is ignored when using
send_messages()
-------------------------------------+-------------------------------------
Reporter: Mike Edmunds | Owner: Mike
Type: | Edmunds
Cleanup/optimization | Status: assigned
Component: Core (Mail) | Version: 5.1
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by Mike Edmunds):

* needs_better_patch: 1 => 0

--
Ticket URL: <https://code.djangoproject.com/ticket/35864#comment:5>

Django

unread,
Oct 30, 2024, 4:45:58 AM10/30/24
to django-...@googlegroups.com
#35864: Warn that EmailMessage.connection option is ignored when using
send_messages()
-------------------------------------+-------------------------------------
Reporter: Mike Edmunds | Owner: Mike
Type: | Edmunds
Cleanup/optimization | Status: assigned
Component: Core (Mail) | Version: 5.1
Severity: Normal | Resolution:
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by Sarah Boyce):

* stage: Accepted => Ready for checkin

--
Ticket URL: <https://code.djangoproject.com/ticket/35864#comment:6>

Django

unread,
Oct 30, 2024, 6:21:03 AM10/30/24
to django-...@googlegroups.com
#35864: Warn that EmailMessage.connection option is ignored when using
send_messages()
-------------------------------------+-------------------------------------
Reporter: Mike Edmunds | Owner: Mike
Type: | Edmunds
Cleanup/optimization | Status: closed
Component: Core (Mail) | Version: 5.1
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by Sarah Boyce <42296566+sarahboyce@…>):

* resolution: => fixed
* status: assigned => closed

Comment:

In [changeset:"17c8ee7e3f7bf400128281b4fb283d7c209ca02b" 17c8ee7]:
{{{#!CommitTicketReference repository=""
revision="17c8ee7e3f7bf400128281b4fb283d7c209ca02b"
Fixed #35864 -- Documented EmailMessage.connection is ignored when using
send_messages().
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/35864#comment:7>

Django

unread,
Oct 30, 2024, 6:38:13 AM10/30/24
to django-...@googlegroups.com
#35864: Warn that EmailMessage.connection option is ignored when using
send_messages()
-------------------------------------+-------------------------------------
Reporter: Mike Edmunds | Owner: Mike
Type: | Edmunds
Cleanup/optimization | Status: closed
Component: Core (Mail) | Version: 5.1
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Sarah Boyce <42296566+sarahboyce@…>):

In [changeset:"ffc67aac1e14360e3c2f3e1de0c64fed289da74c" ffc67aac]:
{{{#!CommitTicketReference repository=""
revision="ffc67aac1e14360e3c2f3e1de0c64fed289da74c"
[5.1.x] Fixed #35864 -- Documented EmailMessage.connection is ignored when
using send_messages().

Backport of 17c8ee7e3f7bf400128281b4fb283d7c209ca02b from main.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/35864#comment:8>
Reply all
Reply to author
Forward
0 new messages