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
Feature: s3 sns notification #6838
Feature: s3 sns notification #6838
Conversation
Codecov Report
@@ Coverage Diff @@
## master #6838 +/- ##
=======================================
Coverage 96.20% 96.20%
=======================================
Files 816 816
Lines 80076 80094 +18
=======================================
+ Hits 77037 77056 +19
+ Misses 3039 3038 -1
Flags with carried forward coverage won't be shown. Click here to find out more.
|
I'm not sure what my changes have to do with terraform failing.
|
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.
Hi @maederm, thanks for the PR! The logic looks good to me.
Can you move the test case into test_s3/test_s3_lambda_integration
? The name is a misnomer, but that file contains all other bucket notification tests
Probably a intermittent failure - don't worry about that. |
Other notifications are tested there as well
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.
LGTM - thank you @maederm!
Hi @maederm, Thank you for contributing to Moto! To show our thanks, we'd like to share some of the donations that we've received with you. PR's like this are the big reason that Moto is as successful as it is - so it's only fair that you, as a contributor, gets to share the spoils. We've created a companion website with more information: Feel free to open a bug or discussion if you run into any problems: |
This is now part of moto >= 4.2.4.dev18 |
Implement missing s3 sns notifications.
I created a new test file because I couldn't find any tests that test the notifications themselves (the ones in test_s3.py test the configuration options, but not the implementation of send_event).
I'm not sure if need to update the function docs from models.py and the s3.rst, just edited both to be sure.