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

Save e-mail contents in easy, machine-readable format #4098

Merged
merged 1 commit into from
Jun 4, 2021
Merged

Save e-mail contents in easy, machine-readable format #4098

merged 1 commit into from
Jun 4, 2021

Conversation

FlorianSW
Copy link
Contributor

When using loclstack for development an even for easier integration/local testing
of interaction with AWS services, a feature is missing to check if an e-mail
was sent correctly.

Issue #2514 was closed with a patch that dumped the e-mail contents to the
log output. However, in most cases, this is not reliably useable for testing,
as it would require to parse the log-stream of localstack.

This commit now adds a feature where a sent e-mail is persisted to the local
file system. It re-uses the specified data directory (DATA_DIR) and falls back
to the temp directory to save e-mails into an ses subdirectory. Each
successfully sent message is saved in a JSON file named after the message ID.
The contents are the Source, Subject, Body, Destinations (To, CCs, BCCs) as
well as the region.

Co-Authored-By: Arne Wohlert arne.wohlert@oss.volkswagen.com

Copy link
Member

@whummer whummer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks awesome, thanks for adding this enhancement @FlorianSW ! Added a few minor comments - please let me know if anything is unclear.. Thanks!

localstack/services/ses/ses_starter.py Outdated Show resolved Hide resolved
localstack/services/ses/ses_starter.py Outdated Show resolved Hide resolved
localstack/services/ses/ses_starter.py Outdated Show resolved Hide resolved
tests/integration/test_ses.py Outdated Show resolved Hide resolved
tests/integration/test_ses.py Outdated Show resolved Hide resolved
tests/integration/test_ses.py Outdated Show resolved Hide resolved
@FlorianSW
Copy link
Contributor Author

Thanks for the fast response and the awesome comments! I'll update the PR tomorrow, hope that's fine with you as well :)

@FlorianSW
Copy link
Contributor Author

One slight hint as well: We just added the save of the e-mail to the text e-mail, not to the raw or templated e-mail. Simply because we use text e-mails only :) Is that ok for you, or should we invest some more time to have this functionality on the other send mail methods as well? 🤔

When using loclstack for development an even for easier integration/local testing
of interaction with AWS services, a feature is missing to check if an e-mail
was sent correctly.

Issue #2514 was closed with a patch that dumped the e-mail contents to the
log output. However, in most cases, this is not reliably useable for testing,
as it would require to parse the log-stream of localstack.

This commit now adds a feature where a sent e-mail is persisted to the local
file system. It re-uses the specified data directory (DATA_DIR) and falls back
to the temp directory to save e-mails into an `ses` subdirectory. Each
successfully sent message is saved in a JSON file named after the message ID.
The contents are the Source, Subject, Body, Destinations (To, CCs, BCCs) as
well as the region.

Co-Authored-By: Arne Wohlert <arne.wohlert@oss.volkswagen.com>
@FlorianSW
Copy link
Contributor Author

Not sure, if I should ask this question here or create an issue :)

Is there a reason why the pipeline (Circle CI) is running the whole test suite once and then running the same test suite while running docker build again? Isn't that a bit duplicated?

From what I got from the comment, I think the "run tests before pushing" is fulfilled with the test run outside of docker already? Background: It would greatly improve build times, especially when someone wants to build the image locally from a git branch which is already tested :)

@whummer
Copy link
Member

whummer commented Jun 4, 2021

Awesome, thanks for the quick turnaround @FlorianSW ! 🚀

@whummer whummer merged commit 7d86a40 into localstack:master Jun 4, 2021
@FlorianSW FlorianSW deleted the f/e-mail-save branch June 4, 2021 10:14
@FlorianSW
Copy link
Contributor Author

Thanks for the fast responses and the merge @whummer :)

dannybster added a commit to dannybster/localstack that referenced this pull request Jul 24, 2021
if an email was sent correctly but relied upon templated emails which the above PR
did not address.

I have essentially duplicated the work done in the above PR but amended it slightly
for the send_templated_email use case i.e. sending a templated email will dump said
email into the DATA_DIR in an ses subdirectory using the id of the sent email as
filename.

The contents of the JSON file are:
- Source
- Template
- TemplateData
- Destinations

e.g.

{
  "Source": "user@example.com",
  "Template": ["hello-world"],
  "TemplateData": ["{\"A key\": \"A value\"}"],
  "Destinations": {
    "ToAddresses": ["success@example.com"],
    "CcAddresses": [],
    "BccAddresses": []
  },
  "Region": "us-east-1"
}
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

Successfully merging this pull request may close these issues.

2 participants