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

S3: add EventBridge notification for Object Tagging #7673

Merged
merged 4 commits into from
May 13, 2024

Conversation

MikiPWata
Copy link
Contributor

@MikiPWata MikiPWata commented May 6, 2024

Added EventBridge notification for S3:PutObectTagging

changes

  • s3/models.py
    • added notifications.send_event
  • tests/test_s3/test_s3_eventbridge_integration.py
    • added test_put_object_tagging_notification for testing S3:PutObectTagging notification
    • modified the assertion for number of events in other tests as implementing notification for S3:PutObjectTagging adds extra event (I guess this is normal?)

@MikiPWata
Copy link
Contributor Author

MikiPWata commented May 6, 2024

Hey @bblommers!
Im trying to add eventbridge notification for S3:PutObjectTagging, but linter wouldn't pass.
Do you have any better approach on how to get the bucket in notification.send_event for put_object_tagging?

@bblommers
Copy link
Collaborator

Hmm.. I don't think there is a clean way to do this @MikiPWata. We know that the bucket_name will realistically always be there, so I think we can just add a # type: ignore to get the linter to pass

@MikiPWata MikiPWata marked this pull request as ready for review May 13, 2024 13:03
@MikiPWata
Copy link
Contributor Author

@bblommers
thanks for the comment!
Do you mind reviewing this?
thanks in advance!

Copy link
Collaborator

@bblommers bblommers left a comment

Choose a reason for hiding this comment

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

LGTM - thank you @MikiPWata!

@bblommers bblommers added this to the 5.0.8 milestone May 13, 2024
@bblommers bblommers merged commit b90a540 into getmoto:master May 13, 2024
38 checks passed
Copy link
Contributor

This is now part of moto >= 5.0.8.dev4

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.

None yet

2 participants