DEP 0018 Dictionary-based EMAIL_PROVIDERS (ticket-35514)#105
Conversation
dbbd52c to
cb21f11
Compare
Move deprecated get_connection() kwargs handling entirely into the backwards compatibility section, and rework it to make the intent clear.
This comment was marked as outdated.
This comment was marked as outdated.
nessita
left a comment
There was a problem hiding this comment.
Submitting my initial set of comments to see if there are any red flags.
nessita
left a comment
There was a problem hiding this comment.
Amazing work, super well laid out, clear and precise. Thank you @medmunds! I'm overall onboard with the proposal as a whole pending some minor fixes. I tried to provide guidance on every 🤔 callout.
Let me know how to progress this!
|
@medmunds I'm happy to be added as Shepherd, and I think we could use number 18 for this and putting it up for review once the small bits are taken care of. What do you think? |
|
@nessita thanks as always for the thorough review and thoughtful suggestions 🙌 I'll update to incorporate all the feedback. (I suspect this may unearth some new questions about I hadn't really thought about handling this as a real DEP; I was just borrowing the format and the repo for discussion. (And I was going to close it unmerged once the details were worked out, since the ticket is already approved.) But now that you suggest it, a DEP makes sense. Thanks for shepherding! Once there's a new draft, I think it's important to get Jacob R's input. (Recognizing the enormous amount of time he's already put into this, I wanted to wait to @ him until there were fewer open questions.) I'll also try to reach out directly to some third-party packages that would be affected (django-mailer, django-celery-email, django-ses, etc.), to make sure they're aware of the proposal and have a chance to weigh in. And it would be helpful to get feedback from people who are actively using the SMTP EmailBackend in production, particularly on some of the deprecations. Maybe a forum post to re-introduce the feature? (My own production experience is with http-API-based backends like django-anymail and django-ses, plus a private fork of django-mailer for queuing and error recovery. So I may have some blind spots around things like get_connection() with username/password overrides.) |
|
@medmunds On a related tanget, I just came across this PR that allows cache backends to "know" their |
Ouroboros! I've been citing the new-features discussion that led to that PR as justification here for providing |
Notable changes: - Moved `fail_silently` from EmailBackend implementations to high-level APIs that call backend.send_messages(). - Renamed `provider` alias string parameter to `using`. (And generally resolved name and type decisions.) - Deprecated all `connection` args. - Made BACKEND optional, defaulting to smtp.EmailBackend. - Eliminated suggestions to use "magic" provider aliases. - Assigned DEP number and added shepherd.
4e9de4f to
58ebef1
Compare
|
I've updated the DEP to incorporate @nessita's feedback. There are still a few open questions (including a few new ones), but I think it's ready for @jrief to take a look if he's available. Based on discussion in this pr, the DEP now proposes moving The |
EmailMessage.send() needs to return the sent count.
Missed in earlier edit.
(sigh)
LilyFirefly
left a comment
There was a problem hiding this comment.
I've given this a full read through and I didn't spot any red flags.
Clean up some TODOs and prep to be a real DEP someday. - Rewrite abstract as an abstract - Split old abstract into history and status, rework - Move motivation earlier - Add myself to the implementation team
…cation Clarify and simplify the compatibility mechanisms.
ca9a991 to
d6a1cab
Compare
`providers.default if using is None else providers[using]` works fine (and still avoids exposing DEFAULT_EMAIL_PROVIDER_ALIAS constant).
tim-schilling
left a comment
There was a problem hiding this comment.
I'm on board with this as is! Thank you @medmunds and @nessita for your work on this. I had a few nitpick comments, some formatting questions, and then provided my opinion around the 🤔 points.
I don't have any substantive requests for change, though if we engage other 3rd party package maintainers, they may have thoughts on the upgrade process.
Incorporate things learned while implementing the feature. Remove suggested implementations from the DEP. (The actual implementation in the reference PR is more accurate now.) Ensure the DEP specifies expected behavior. Rework "Upgrading EmailBackend implementations" to provide more flexibility for third-party backends that might reasonably want to keep existing settings alongside EMAIL_PROVIDERS OPTIONS support. Clean up the document structure and move some supplemental material later in the document for readability. Clean up punctuation, wrapping and stray whitespace.
There was a problem hiding this comment.
Why "providers"? I'd honestly prefer a technical term that is hyperscaler agnostic. We also don't do datastore_providers but DATABASES.
There are still people who run their own SMTP servers. They might still run different configurations, even on a single server.
I admit, this is highly subjective, but I thought I at least raise the concern.
|
Please check the discussion here: https://code.djangoproject.com/ticket/35514 We came up with this name during the sprints at Django Con Europe 2024 (Carton Gibson, Natalia Bidart, Jacob Rief). |
In said thread, I see a multitude of people raising the same concern, but no resolution. And I don't warm up to the idea of it referring to "email service providers".
The closest technical term I can think of would be I am really hesitant to open the doors for non-technical terms that describe institutions. IMHO, it sets a negative precedent and leaves a strange aftertaste. |
|
Side note: every big provider, from AWS, Google, Twilio, or Azure, refers to their services as SMTP relays. |
Thank you everyone for revisiting this. It's good to challenge decisions with fresh eyes. We discussed the settings name at length, and the key distinction that is worth mentioning first is that settings like In practice, this setting represents a set of delivery strategies rather than a single mechanism. While SMTP may be used at the boundary, from the Django user's POV there are many valid approaches, including API-based backends that do not expose SMTP at all. In that sense, this setting represents a collection of configurations for different delivery strategies (SMTP, HTTP APIs, gateways, etc.), which do not map cleanly to a single "relay" concept. "Relay" is also quite SMTP-specific and less approachable as a general term, whereas Other options we explored:
Anyway, if from django.core.mail import senders
senders.default.send_messages([...]) |
|
I like the simplicity of |
|
As a perfectionist, I'm tempted to revisit the naming bikeshed. As a perfectionist with a looming 6.1 deadline, I'm inclined to stick with EMAIL_PROVIDERS.
It looks like I represent fully 50% of that multitude. I initially expressed concern about the name, but in a later comment revised my position and supported I think If we had to change it (which, again, I don't think is advisable if we want to land this in 6.1):
|
|
I would also stick with My second choice would be |
|
I also like I can take ownership of finalizing/driving the rename once the feature is complete (so we focus on that). |
|
Stupid question: We are always talking about e-mails, right? Not SMS or RCS, right? Because emails are ALWAYS transmitted via SMTP, even if you use a different protocol to speak to your relay. However, I understand that “relay” is a rather technical term used by DevOps folk. Back to RCS, I mean, do we want to support RCS backends in the future? This would be super cool! Especially since the EU is enforcing client interoperability. How cool would sending iMessage or WhatsApp messages be natively inside Django 🤯 Certainly outside the scope for this poor proposal, but with this in mind:
Oh, and sorry for reopening the discussion. Naming things and cache invalidation remain the hardest things in computer science… 🤷♂️ |
Jacob R mentioned something similar in the ticket. Some sort of "integrated communications" (sorry for the marketing-speak) does seem like a natural extension of core.mail and contrib.messages. I was about to suggest prototyping it in a third-party package, then TIL django-notifs. Fwiw, they call their pluggable communications channels "providers."
It's a useful discussion. The DEP should probably mention the rationale for the naming (whatever we end up with) and outline the alternatives that were considered. (Also… is there one other hardest thing in computer science? I always lose count.) |
|
Thank you everyone, so much, for your feedback and thoughtful comments. After serious deliberation and hands-on work seeing how the various options would look and work in the code, Mike and I will proceed with the rename of the settings |
|
This DEP will be updated in the next 7-10 days to reflect the recent (and minor other than the rename) changes following the implementation. Once the DEP text is updated, I'll propose the PR to move it to the |
jacobtylerwalls
left a comment
There was a problem hiding this comment.
@LilyFirefly do you have permission to merge? We can fix the MAILERS name change in a follow-up.
|
I am in the process of updating the draft DEP for the renaming, and will open a new PR for that soon. |
📄 Read the formatted draft DEP 0018, latest revision.
See ticket-35514#comment:24
Django ticket-35514 will implement a dictionary-based
EMAIL_PROVIDERSsetting, similar toCACHES,DATABASES,STORAGESandTASKS. The ticket was approved following discussion on the django-developers list and at DjangoCon Europe 2024 sprints.The purpose of this DEP is to facilitate discussion and decisions on the proposed API and related deprecations.