-
Notifications
You must be signed in to change notification settings - Fork 6
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
(self host 1/?): allow users to configure their own email service #972
Conversation
…r allow users to disable email
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice, though I do think we should drop the MAILGUN_
prefix. I can do it in a separate PR if you'd like since I don't mind messing with terraform
MAILGUN_SMTP_PORT=587 | ||
MAILGUN_SMTP_USERNAME="[email protected]" | ||
MAILGUN_SMTP_PASSWORD="your-password" | ||
MAILGUN_SMTP_FROM="[email protected]" # technically optional, but you will almost definitely need to set this, as it will use `[email protected]` by default. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Might want to add this longer comment to the above "From" configs too. Though honestly I think we should make it and name
required instead of having our defaults hardcoded.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i agree. although i think this should be a community level config, not an installation wide one, which i why i left it for now
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah that sounds right to me! We shouldn't let people send arbitrary emails under our name 😟
Co-authored-by: Kalil Smith-Nuevelle <[email protected]>
Issue(s) Resolved
Partially #954
Note
I will base the other self host prs on this one, but that does not mean we necessarily need to merge it!
If for some reason we do not want to allow users to use different email providers (?)
This allows users to pick a different email provider somewhat easily.
High-level Explanation of PR
Adds some documentation and presets on how to use different email providers with platfrom.
Additionally, this makes the
MAILGUN_
env vars optional for self hosting. This will then only throw an error when you try to use the email client.I don't want users to NEED to configure an email client. They should probably, but it should be possible to run one without.
I also considered adding an option to use the native
sendmail
ormsmtp
or equivalent, but I'm not sure we should encourage that.Ideally I would have made this more typesafe by making our config a discriminated union, eg
However this does not seem possible, so instead i have this weird wrapper
platform/core/lib/env/env.mjs
Lines 6 to 15 in c3bee15
Test Plan
MAILGUN
variables as suchplatform/self-host/README.md
Lines 32 to 45 in 60758c9
Screenshots (if applicable)
Notes
the variables probably should not be prefixed by
MAILGUN_
, but i don't like messing with theterraform
config.