-
Notifications
You must be signed in to change notification settings - Fork 453
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
Added optional email cleanup on S3 at end of process #73
base: master
Are you sure you want to change the base?
Conversation
53348d8
to
c32a6e1
Compare
Looks good. But probably unnecessary, because you can set on bucket LifeCycle Management to expire object. For example: you can set to delete received ecah email after 1 day. |
Yeah I saw that after developing the feature.. 😞 |
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.
You move the steps
below, and you push one item to it here. That broke extendability of this array by overrides.steps
.
I suggest: revert changes in exports.handler
method, just add exports.emailsCleanupOnS3
to array steps
directly without conditions. And move evaluation of the data.config.emailsCleanupOnS3
inside to exports.emailsCleanupOnS3
function.
If you can, please update README.md documenation (especially paragraph 8) to complete your PR.
Please revert your change of package.json
too. Versioning of releases should be privilege to @arithmetric when he will merge it.
TIP: just add new commit (no ammend&force-push), @arithmetric can squash it during merge.
Back to our discussion: you are right, but my experience is: Set LifeCycle to 1 week at least. It is good for keep stack cleaned, but still ready to debug when some some is forwarded, but looks corrupted. When you want to erase is manually after success redirect, is appropriate to turn on versioning to able recovery deleted messages for unexpected fails and then must it clear it again by LifeCycle - OK, but it looks (for me) as the same concept, but more complex. But you have right, deleting of bucket objects can be useful for some users. Thanks you for contribution 👍 |
And completed documentation
Updated PR from recommendations. |
Hi, |
No description provided.