Skip to content
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

AWS SES configuration #98

Closed
memsbdm opened this issue Mar 30, 2024 · 4 comments
Closed

AWS SES configuration #98

memsbdm opened this issue Mar 30, 2024 · 4 comments

Comments

@memsbdm
Copy link
Contributor

memsbdm commented Mar 30, 2024

Package version

9.2.1

Describe the bug

When configuring SES as mailer, environment variables were not properly set.

Examples:

  • AWS_ACCESS_KEY instead of SES_ACCESS_KEY_ID
  • SES_ACCESS_SECRET instead of SES_SECRET_ACCESS_KEY, ...

PR #97 is solving issues. Also in docs the PR #67 is changing the example.

Reproduction repo

No response

@memsbdm memsbdm mentioned this issue Mar 30, 2024
6 tasks
@Julien-R44
Copy link
Member

I would say we should do the reverse. I mean, change the names of the environment variables

I know this was the case on V5: we had variables SES_ACCESS_KEY_ID etc..
But I've always found it annoying: in general, if you're using SES, you're probably using other AWS services. In particular S3, or SQS. And so I found myself constantly renaming these variables in a more generic way.

So, I'd be more in favor of keeping

AWS_ACCESS_KEY_ID=
AWS_SECRET_ACCESS_KEY=
AWS_REGION=

rather than

SES_ACCESS_KEY_ID=
SES_SECRET_ACCESS_KEY=
SES_REGION=

That way, these same variables can be used directly in the configuration of other things, like Drive for example. Without having to rename SES_ACCESS_KEY_ID to AWS_ACCESS_KEY_ID and so on. Because you probably use the same keys

@memsbdm
Copy link
Contributor Author

memsbdm commented Mar 30, 2024

I'd thought about it, but I thought it might be confusing if some people were using several services with different keys.
Nevermind, I updated the PR with AWS_!

@memsbdm
Copy link
Contributor Author

memsbdm commented May 4, 2024

Hey @Julien-R44 can you merge the PR? 😃

@Julien-R44
Copy link
Member

Done! Thanks for fixing that

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants