Implement a system check for no default MAILERS configuration

[Jacob Walls]

Because the deprecated mail settings will stop working in Django 7.0, this could lead to data loss if someone misses the deprecation warnings and fails to migrate to MAILERS.

For a similar scenario with STORAGES, on the forum we ​appeared to agree that in retrospect, we should have implemented a temporary system check in core.checks.compatibility despite some disagreement about what a more robust, permanent solution might look like for retiring settings.

An example where this was implemented before is in 354acd04af524ad82002b903df1189581c51cabe, although it's not a perfect analogue because that check only checked for conflicts between old & new settings--which we do have right now in 6.1, raising a runtime ImproperlyConfigured--not the presence of the old settings at all.

(Interestingly, the ImproperlyConfigured is swallowed by manage.py shell...)

Anyway, I think we should consider adding a system check during the deprecation period.

[Mike Edmunds]

Note DEP 0018 contemplates two similar ​MAILERS-related system checks as future work.

The "Missing "default" alias in MAILERS" check would catch this starting in 7.0 (because MAILERS defaults to {} starting in 7.0).

[Mike Edmunds]

[Jacob, this would be release blocker for 7.0, right? Not for 6.1.]

[Jacob Walls]

Ah, I didn't realize that was the plan. That would be sufficient, I think.

[Natalia Bidart]

Thanks!

[Mike Edmunds]

So the goal is a system check that warns if "default" not in settings.MAILERS. Couple of questions:

• Projects that haven't configured MAILERS because they don't send mail will need to silence the check. Are we OK with that? 🤔
• None of the ​built-in tags seem to apply. I'm thinking we should add a tag for mail?

Other notes:

• This check could be added immediately, but before 7.0 the test will need to be hasattr(settings, "MAILERS") and … (because MAILERS doesn't exist by default until 7.0).
• Suggested msg: "There is no 'default' configuration in your MAILERS setting."
• Suggested hint:
  • If MAILERS is empty: "Sending email will cause an error."
  • If MAILERS has other keys: "Sending email without specifying 'using' will cause an error."

[Jacob Walls]

Replying to 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.

[Mike Edmunds]

Replying to Jacob Walls:

Replying to 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 ​system check to detect a missing "default" entry in MAILERS
• Avoiding noisy (deprecation) warnings ​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.

[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.

[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.

[Mike Edmunds]

Thanks, Natalia. I agree with everything you said.

Replying to 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.

[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.

[nessita <124304+nessita@…>]

In 99a14b6b:

Fixed #37161 -- Warned via system check on missing "default" entry in MAILERS setting.

Thanks Mike Edmunds for reviews.

Co-authored-by: Natalia <124304+nessita@…>

[Natalia <124304+nessita@…>]

In 692d567e:

[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+nessita@…>

Backport of 99a14b6be85793f3371779cd9bf6fb9d9247f475 from main.