Replace use_mailpit option by mail_catcher#6732
Conversation
Why make it a separate option, then? |
From my point of view, to provide the least amount of friction (keeping it backwards-compatible) and follow existing conventions of having to pick one tool. I don't mind refactoring this into a single option, but that would be a minor breaking change for existing configs that set |
|
We've done these kind of changes before so if we add mailtrap, I would combine it into a single option to choose between mailpit and mailtrap. If it doesn't make sense to pick them together, so let's make it impossible in the questionnaire to pick them... I'm saying if, because I'm not sure it's worth introducing another choice the user has to make... Folks using this template may not have heard of neither mailpit nor mailtrap, so at the point of prompting they would have to potentially go learn what each does and decide which one to pick if they want this kind of solution. Right now, they only have to decide if they want local SMTP or not. IMO this is an area where we should be opiniated and pick the best solution of the 2, based on feature easy to use and licensing. If they are equal in all these areas, the status quo wins. If the current solution isn't maintained, then that's an incentive to switch. Offering more options or choices always adds complexity: longer CI run, more cognitive overhead on the users, etc... Thats my 2 cents, I'm not convinced about this at the moment. |
|
I've made the change so that there is one choice for mail_catcher (None / Mailpit / Mailtrap Local), defaulting to None, so you can't pick both by construction.
Is this additional choice worthwhile? I'd say it's more of the "what does the team already have" than "which one is better". There are multiple options of the "this tool is better than the other" type, e.g., mail_service, cloud provider, CI and in those cases, it's explicitly stated what the default is. Local catcher seems to be in the same boat – teams that are already running Mailtrap somewhere may find it easier to use the same catcher in their dev environment. But as the default is None, people who don't need local SMTP just leave it there and forget about it, as the choice matters only to people who have already decided to use the catcher. Right now, both of them might be similar on features and licensing — but that's kind of my point: when two tools are comparable, 'which is objectively best' stops being the useful question, and 'which does the team already run' takes over. That's the same reasoning behind offering a choice for mail_service or CI rather than hard-coding one. |
Thanks, that looks better.
I see, that's a fair reasoning, when taking into consideration advanced users who know what they want: experienced devs, or established teams. I was more thinking about less experienced folks (in general or just newer to the Django ecosystem) coming to cookiecutter-django without knowing that ahead of time.
Yes, I think the difference with these other examples is that I see them as much more important to justify the additional friction of front-loading this choice via an option... For mail catcher, I don't think the tool matters much but perhaps it's because I don't use them myself 🤷🏻
Another good point, this is pretty much how I would use it. Also, as we already have an option for that, we "just" trading one for another, so it's not as bad as if we were adding an entirely new one. |
| Mailpit | ||
| ~~~~~~~ | ||
|
|
||
| When developing locally you can go with Mailpit_ for email testing provided ``use_mailpit`` was set to ``y`` on setup. To proceed, | ||
| When developing locally you can go with Mailpit_ for email testing provided ``mail_catcher`` was set to ``Mailpit`` on setup. To proceed, | ||
|
|
||
| #. make sure ``<project_slug>_local_mailpit`` container is up and running; | ||
|
|
||
| #. open up ``http://127.0.0.1:8025``. | ||
|
|
||
| .. _Mailpit: https://github.com/axllent/mailpit/ | ||
|
|
||
| Mailtrap Local | ||
| ~~~~~~~~~~~~~~ | ||
|
|
||
| When developing locally you can go with `Mailtrap Local`_ for email testing provided ``mail_catcher`` was set to ``Mailtrap Local`` on setup. To proceed, | ||
|
|
||
| #. make sure ``<project_slug>_local_mailtrap`` container is up and running; | ||
|
|
||
| #. open up ``http://127.0.0.1:3550``. |
There was a problem hiding this comment.
Should we group these 2 under a "Mail catcher" section? Same for the local dev without Docker
There was a problem hiding this comment.
Yes, that makes sense and aligns with overall changes in this PR. Grouped under Mail catcher.
browniebroke
changed the title
use_mailpit option by mail_catcher
c50a732 to
043064a
Compare
Adds a use_mailtrap_local cookiecutter prompt as a peer alternative to the existing use_mailpit prompt (not a replacement). Setting it to y wires Mailtrap Local into docker-compose.local.yml and the local Django email settings the same way use_mailpit does today (SMTP on 3535, web UI on 3550). The two options are mutually exclusive: the pre-generation hook aborts if both are enabled. Docs, README, .gitignore and the generation test matrix are updated to mirror the existing Mailpit setup.
The mailtrap-local image runs as a non-root user and writes its SQLite database and secret key under /var/lib/mailtrap-local, a directory that does not exist in the image. Without a writable mount there the container fails to start with "unable to open database file". A tmpfs mount is writable by the non-root user and keeps the service ephemeral, matching the volume-less Mailpit setup.
… option Replace the two mutually-exclusive booleans with one choice-list option (None / Mailpit / Mailtrap Local), defaulting to None. This keeps local SMTP opt-in as before, makes picking both impossible by construction, and removes the pre-gen mutual-exclusivity check that is no longer needed.
Nest the Mailpit and Mailtrap Local subsections under a Mail catcher heading in both the local and Docker local-development docs, matching the single mail_catcher option.
043064a to
19d401a
Compare
2 participants
Description
Adds a
use_mailtrap_localoption as a peer alternative to the existinguse_mailpitprompt. Same docker-compose drop-in pattern; different tool choice for developers who prefer Mailtrap Local. Settinguse_mailtrap_local=ywires Mailtrap Local intodocker-compose.local.ymland the local Django email settings the same wayuse_mailpit=ydoes today (SMTP on3535, web UI on3550).Checklist:
Rationale
This adds a peer local-email-testing option; it does not change the existing Mailpit default. Mailtrap Local is an MIT-licensed, single self-contained Go binary that catches local SMTP and exposes a web UI + REST API — the same drop-in role Mailpit fills today. It gives developers who prefer it a first-class local option while leaving Mailpit users completely unaffected. The two options are mutually exclusive (enforced in the pre-gen hook), and the docs, tests,
.gitignore, and docker-compose wiring mirror the existing Mailpit setup.Changes
cookiecutter.json— newuse_mailtrap_localprompt (defaultn), peer touse_mailpithooks/pre_gen_project.py— aborts generation if both catchers are enableddocker-compose.local.yml—mailtrap-localservice (with atmpfsmount so the non-root image can start) +depends_onwiring for django/celeryworker/celerybeatconfig/settings/local.py—EMAIL_HOST/EMAIL_PORTfor docker and non-docker (port3535).gitignore— ignore themailtrap-localbinary (non-docker)tests/test_cookiecutter_generation.py— supported combinations + mutual-exclusion case