Re: [Django] #37161: Implement a system check for no default MAILERS configuration

[Django]

#37161: Implement a system check for no default MAILERS configuration
-------------------------------------+-------------------------------------
Reporter: Jacob Walls | Owner: Bader
| Eddine Benhirt
Type: New feature | Status: assigned
Component: Core (Mail) | Version: dev
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 Bader Eddine Benhirt):

* has_patch: 0 => 1
* owner: (none) => Bader Eddine Benhirt
* status: new => assigned

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

[Django]

#37161: Implement a system check for no default MAILERS configuration

-----------------------------+------------------------------------
Reporter: Jacob Walls | Owner: (none)

Type: New feature | Status: assigned
Component: Core (Mail) | Version: dev
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 Bader Eddine Benhirt):

* has_patch: 1 => 0
* owner: Bader Eddine Benhirt => (none)

--
Ticket URL: <https://code.djangoproject.com/ticket/37161#comment:9>

[Django]

#37161: Implement a system check for no default MAILERS configuration
-----------------------------+------------------------------------
Reporter: Jacob Walls | Owner: (none)
Type: New feature | Status: assigned
Component: Core (Mail) | Version: dev
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 Bader Eddine Benhirt):

* has_patch: 0 => 1

--
Ticket URL: <https://code.djangoproject.com/ticket/37161#comment:10>

[Django]

#37161: Implement a system check for no default MAILERS configuration

-------------------------------------+-------------------------------------
Reporter: Jacob Walls | Owner: Bader
| Eddine Benhirt

Type: New feature | Status: assigned
Component: Core (Mail) | Version: dev
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 Bader Eddine Benhirt):

* owner: (none) => Bader Eddine Benhirt

--
Ticket URL: <https://code.djangoproject.com/ticket/37161#comment:11>

[Django]

#37161: Implement a system check for no default MAILERS configuration
-------------------------------------+-------------------------------------
Reporter: Jacob Walls | Owner: Bader
| Eddine Benhirt
Type: New feature | Status: assigned
Component: Core (Mail) | Version: dev
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 Mike Edmunds):

* needs_better_patch: 0 => 1

--
Ticket URL: <https://code.djangoproject.com/ticket/37161#comment:12>

[Django]

#37161: Implement a system check for no default MAILERS configuration
-------------------------------------+-------------------------------------
Reporter: Jacob Walls | Owner: Bader
| Eddine Benhirt
Type: New feature | Status: assigned
Component: Core (Mail) | Version: dev
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
-------------------------------------+-------------------------------------

Comment (by Jacob Walls):

Replying to [comment:6 Mike Edmunds]:
> * Projects that haven't configured `MAILERS` because they don't send
mail will need to silence the check. Are we OK with that? 🤔

I guess the alternative would be to limit the check to the circumstance
where at least one deprecated `EMAIL_*` setting is present, like we do for
the deprecation warning if I recall correctly.
--
Ticket URL: <https://code.djangoproject.com/ticket/37161#comment:13>

[Django]

#37161: Implement a system check for no default MAILERS configuration
-------------------------------------+-------------------------------------
Reporter: Jacob Walls | Owner: Bader
| Eddine Benhirt
Type: New feature | Status: assigned
Component: Core (Mail) | Version: dev
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 Mike Edmunds):

* cc: Natalia Bidart (added)

Comment:

Replying to [comment:13 Jacob Walls]:

> Replying to [comment:6 Mike Edmunds]:
> > * Projects that haven't configured `MAILERS` because they don't send
mail will need to silence the check. Are we OK with that? 🤔
>
> I guess the alternative would be to limit the check to the circumstance
where at least one deprecated `EMAIL_*` setting is present, like we do for
the deprecation warning if I recall correctly.

A few thoughts:

* In Django 7.0, if `MAILERS` is not defined, any attempt to send mail
will raise a `MailerDoesNotExist` error. There's no risk of ''silent''
data loss (like what seems to have been the concern with `STORAGES`).
* #35674 (accepted) will add a generalized system check for all (or most?)
previously-deprecated settings once they are removed. So the `EMAIL_*`
settings could be added to that check when they are removed in 7.0. (I
don't think we should create a separate check just for old email
settings.)
* #37166 (awaiting triage) is the other system check suggested in the DEP,
to warn when a development-only email backend is configured as the default
mailer in production. Now that the console backend is the default in the
new project template. I think that's a more-likely scenario for easily-
overlooked data loss (in some broad sense of "loss", e.g., password reset
emails that end up printed to a log rather than sent).

Some relevant discussions during feature development (cc Natalia):
* Having a [https://github.com/django/deps/pull/105#discussion_r2807907290
system check to detect] a missing `"default"` entry in `MAILERS`
* Avoiding noisy (deprecation) warnings
[https://github.com/django/django/pull/21052#discussion_r3156662085 for
projects that don't use mail]

Perhaps this "no default mailer" check should only run if `MAILERS` is
''explicitly'' defined in settings.py—not warn if it's an empty dict
because of the Django 7.0 default value.
--
Ticket URL: <https://code.djangoproject.com/ticket/37161#comment:14>

[Django]

#37161: Implement a system check for no default MAILERS configuration
-------------------------------------+-------------------------------------
Reporter: Jacob Walls | Owner: Bader
| Eddine Benhirt
Type: New feature | Status: assigned
Component: Core (Mail) | Version: dev
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
-------------------------------------+-------------------------------------

Comment (by Natalia Bidart):

Thank you everyone for the discussion. A few comments from my side:

> Jacob, this would be a release blocker for 7.0, right? Not for 6.1.

I actually think we should try to land this in 6.1 as a warning-level
system check, not an error. The goal is to give users advance notice and
encourage migration well before 7.0, where the deprecated settings are
removed.

> #35674 (accepted) will [...]

While I agree with Mike's assessment of that ticket, I am not expecting
much short-term progress there given the ongoing disagreement around the
implementation. From my perspective, it is effectively stalled for now, so
I would not rely on it as part of the `MAILERS` migration story.

> Avoiding noisy (deprecation) warnings [...]

I feel strongly that we should not "bother" projects (or reusable apps)
that do not use email at all. A project that never sends email should not
need to configure `MAILERS`, and it should not need to silence warnings or
system checks related to email. Likewise, reusable apps should not be
encouraged to define email configuration merely to satisfy a check.

For that reason, I would support a check only when `MAILERS` is explicitly
defined by the project. At that point, Django can reasonably assume the
project intends to use the email framework and validate the configuration
accordingly. In particular, warning when `MAILERS` is defined but does not
contain a `"default"` alias seems reasonable to me.

I am less convinced that we should add a separate system check for
projects still using explicit `EMAIL_*` settings, since those users should
already receive deprecation warnings. If we do want to add check for
these, perhaps these are the one that should target 7.0 only.

> None of the built-in tags seem to apply. I'm thinking we should add a
tag for mail?

+1 to adding a dedicated `mail` tag.

> #37166 (awaiting triage) is the other system check suggested in the DEP

I think we could fix both in this same ticket, unclear if we really need
two tickets, but I don't object either.

Hopefully this helps answers some of the questions.
--
Ticket URL: <https://code.djangoproject.com/ticket/37161#comment:15>

[Django]

#37161: Implement a system check for no default MAILERS configuration
-------------------------------------+-------------------------------------
Reporter: Jacob Walls | Owner: Bader
| Eddine Benhirt
Type: New feature | Status: assigned
Component: Core (Mail) | Version: dev
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 Bader Eddine Benhirt):

* needs_better_patch: 1 => 0

--
Ticket URL: <https://code.djangoproject.com/ticket/37161#comment:16>

[Django]

#37161: Implement a system check for no default MAILERS configuration
-------------------------------------+-------------------------------------
Reporter: Jacob Walls | Owner: Bader
| Eddine Benhirt
Type: New feature | Status: assigned
Component: Core (Mail) | Version: dev
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
-------------------------------------+-------------------------------------

Comment (by Bader Eddine Benhirt):

Thanks everyone for the clarification.

I updated the PR to follow the direction discussed here. The check now
remains limited to projects that explicitly define MAILERS, and the
files/imports have been renamed to match the new mail check category.

I also ran the focused tests and the check framework tests locally.
--
Ticket URL: <https://code.djangoproject.com/ticket/37161#comment:17>

[Django]

#37161: Implement a system check for no default MAILERS configuration
-------------------------------------+-------------------------------------
Reporter: Jacob Walls | Owner: Bader
| Eddine Benhirt
Type: New feature | Status: assigned
Component: Core (Mail) | Version: dev
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 Mike Edmunds):

* needs_better_patch: 0 => 1

Comment:

Thanks, Natalia. I agree with everything you said.

Replying to [comment:15 Natalia Bidart]:
> […] For that reason, I would support a check only when `MAILERS` is

explicitly defined by the project. At that point, Django can reasonably
assume the project intends to use the email framework and validate the
configuration accordingly. In particular, warning when `MAILERS` is
defined but does not contain a `"default"` alias seems reasonable to me.

[…]

Once the current PR is updated to reflect this, I think it will be ready.
--
Ticket URL: <https://code.djangoproject.com/ticket/37161#comment:18>

[Django]

#37161: Implement a system check for no default MAILERS configuration
-------------------------------------+-------------------------------------
Reporter: Jacob Walls | Owner: Bader
| Eddine Benhirt
Type: New feature | Status: assigned
Component: Core (Mail) | Version: dev
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 Bader Eddine Benhirt):

* needs_better_patch: 1 => 0

--
Ticket URL: <https://code.djangoproject.com/ticket/37161#comment:19>

[Django]

#37161: Implement a system check for no default MAILERS configuration
-------------------------------------+-------------------------------------
Reporter: Jacob Walls | Owner: Bader
| Eddine Benhirt
Type: New feature | Status: assigned
Component: Core (Mail) | Version: dev
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
-------------------------------------+-------------------------------------

Comment (by Bader Eddine Benhirt):

Thanks, I updated the PR to use settings.is_overridden("MAILERS"), so the
warning is only emitted when MAILERS is explicitly defined by the project.
I also removed the temporary RemovedInDjango70Warning comment and added a
test for the case where MAILERS is not explicitly defined.
--
Ticket URL: <https://code.djangoproject.com/ticket/37161#comment:20>

[Django]

#37161: Implement a system check for no default MAILERS configuration
-------------------------------------+-------------------------------------
Reporter: Jacob Walls | Owner: Bader
| Eddine Benhirt
Type: New feature | Status: assigned
Component: Core (Mail) | Version: dev
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 Mike Edmunds):

* stage: Accepted => Ready for checkin

--
Ticket URL: <https://code.djangoproject.com/ticket/37161#comment:21>

[Django]

#37161: Implement a system check for no default MAILERS configuration
-------------------------------------+-------------------------------------
Reporter: Jacob Walls | Owner: Bader
| Eddine Benhirt
Type: New feature | Status: assigned
Component: Core (Mail) | Version: dev
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 Mike Edmunds):

* needs_better_patch: 0 => 1
* stage: Ready for checkin => Accepted

--
Ticket URL: <https://code.djangoproject.com/ticket/37161#comment:22>

[Django]

#37161: Implement a system check for no default MAILERS configuration
-------------------------------------+-------------------------------------
Reporter: Jacob Walls | Owner: Bader
| Eddine Benhirt
Type: New feature | Status: assigned
Component: Core (Mail) | Version: dev
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 Bader Eddine Benhirt):

* needs_better_patch: 1 => 0

--
Ticket URL: <https://code.djangoproject.com/ticket/37161#comment:23>

[Django]

#37161: Implement a system check for no default MAILERS configuration
-------------------------------------+-------------------------------------
Reporter: Jacob Walls | Owner: Bader
| Eddine Benhirt
Type: New feature | Status: assigned

Component: Core (Mail) | Version: 6.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 Mike Edmunds):

* stage: Accepted => Ready for checkin

* version: dev => 6.1

--
Ticket URL: <https://code.djangoproject.com/ticket/37161#comment:24>

[Django]

#37161: Implement a system check for no default MAILERS configuration
-------------------------------------+-------------------------------------
Reporter: Jacob Walls | Owner: Bader
| Eddine Benhirt

Type: New feature | Status: closed

Component: Core (Mail) | Version: 6.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 nessita <124304+nessita@…>):

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

Comment:

In [changeset:"99a14b6be85793f3371779cd9bf6fb9d9247f475" 99a14b6b]:
{{{#!CommitTicketReference repository=""
revision="99a14b6be85793f3371779cd9bf6fb9d9247f475"
Fixed #37161 -- Warned via system check on missing "default" entry in
MAILERS setting.

Thanks Mike Edmunds for reviews.

Co-authored-by: Natalia <124304+...@users.noreply.github.com>
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/37161#comment:25>

[Django]

#37161: Implement a system check for no default MAILERS configuration
-------------------------------------+-------------------------------------
Reporter: Jacob Walls | Owner: Bader
| Eddine Benhirt
Type: New feature | Status: closed
Component: Core (Mail) | Version: 6.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 Natalia <124304+nessita@…>):

In [changeset:"692d567ed31146930dc95b2a401c0e5bb1c79081" 692d567e]:
{{{#!CommitTicketReference repository=""
revision="692d567ed31146930dc95b2a401c0e5bb1c79081"
[6.1.x] Fixed #37161 -- Warned via system check on missing "default" entry

in MAILERS setting.

Thanks Mike Edmunds for reviews.

Co-authored-by: Natalia <124304+...@users.noreply.github.com>

Backport of 99a14b6be85793f3371779cd9bf6fb9d9247f475 from main.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/37161#comment:26>