Skip to content

feat(data-classes): adds support for S3 event notifications through EventBridge #1990

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

Conversation

ivica-k
Copy link
Contributor

@ivica-k ivica-k commented Mar 8, 2023

#1656

Summary

Changes

adds dataclasses/events support for S3 event notifications through EventBridge

Note: Relies on PR #1982 being merged since they share events. I'll happily rebase once #1982 is merged.

Note2: Pay special attention to me overriding mypy checks with # type: ignore[override]

User experience

    from aws_lambda_powertools.utilities.data_classes import event_source, S3EventBridgeNotificationEvent

    @event_source(data_class=S3EventBridgeNotificationEvent)
    def lambda_handler(event: S3EventBridgeNotificationEvent, context):
        bucket_name = event.detail.bucket.name
        file_key = event.detail.object.key

Checklist

If your change doesn't seem to apply, please leave them unchecked.

Is this a breaking change?

RFC issue number:

Checklist:

  • Migration process documented
  • Implement warnings (if it can live side by side)

Acknowledgment

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

Disclaimer: We value your time and bandwidth. As such, any pull requests created on non-triaged issues might not be successful.

@ivica-k ivica-k requested a review from a team as a code owner March 8, 2023 19:05
@ivica-k ivica-k requested review from am29d and removed request for a team March 8, 2023 19:05
@boring-cyborg boring-cyborg bot added documentation Improvements or additions to documentation tests labels Mar 8, 2023
@pull-request-size pull-request-size bot added the size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. label Mar 8, 2023
@ivica-k ivica-k changed the title Feat(event source) 1656 s3 event eventbridge dataclass feat(data-classes): adds support for S3 event notifications through EventBridge Mar 8, 2023
@heitorlessa heitorlessa linked an issue Mar 10, 2023 that may be closed by this pull request
2 tasks
@github-actions github-actions bot added the feature New feature or functionality label Mar 10, 2023
@github-actions
Copy link
Contributor

No related issues found. Please ensure there is an open issue related to this change to avoid significant delays or closure.

@github-actions github-actions bot added do-not-merge need-issue PRs that are missing related issues labels Mar 10, 2023
@ivica-k
Copy link
Contributor Author

ivica-k commented Mar 10, 2023

I can't reproduce the code quality failing on Python 3.7 locally. Added # noqa: A003 to try and silence and locally it passes with make pr.

Copy link
Contributor

@heitorlessa heitorlessa left a comment

Choose a reason for hiding this comment

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

looks great - you're accidentally including parser commits ;) Please ping after removing it and I'll pull to test it, push any necessary changes, and merge it

@heitorlessa heitorlessa removed do-not-merge need-issue PRs that are missing related issues labels Mar 10, 2023
@heitorlessa
Copy link
Contributor

I can't reproduce the code quality failing on Python 3.7 locally. Added # noqa: A003 to try and silence and locally it passes with make pr.

Looking

@heitorlessa heitorlessa removed the request for review from am29d March 10, 2023 08:37
@heitorlessa
Copy link
Contributor

hmmm that's odd. I thought it was dependency lock being out of sync locally vs CI, but even after rebasing it's causing that problem in CI only -- it's almost as if git checkout is dirty. I'll do some more digging

@ivica-k
Copy link
Contributor Author

ivica-k commented Mar 10, 2023

looks great - you're accidentally including parser commits ;) Please ping after removing it and I'll pull to test it, push any necessary changes, and merge it

Events JSON files are needed in both PRs (#1982 and this one) so I created this branch from ivica-k:feat(event-source)-1656-s3-event-eventbridge. Once the parser utility from #1982 is merged I'll rebase this branch on develop.

Unless you have a better idea on how to approach this... :)

@heitorlessa
Copy link
Contributor

looks great - you're accidentally including parser commits ;) Please ping after removing it and I'll pull to test it, push any necessary changes, and merge it

Events JSON files are needed in both PRs (#1982 and this one) so I created this branch from ivica-k:feat(event-source)-1656-s3-event-eventbridge. Once the parser utility from #1982 is merged I'll rebase this branch on develop.

Unless you have a better idea on how to approach this... :)

Oh I was referring to these: https://github.com/awslabs/aws-lambda-powertools-python/pull/1990/files#diff-0640cb056db90940195cc25741ffec24b333d690b27936650a16f163bd74239cR61

If it's easier to merge the parser one first and rebase, let's go for it then; I don't mind. Got two upcoming meetings but should get the parser reviewed and merged today

@heitorlessa
Copy link
Contributor

heitorlessa commented Mar 10, 2023

Confirmed it's the GH Action pulling a dirty commit that isn't in this history: b9d1c6364cb857c3c23f670f15fc64d81a82d488.

I'll pause this for now, work on the Parser PR, then we do git fixups to get GH Action to behave

image

@ivica-k ivica-k force-pushed the feat(event-source)-1656-s3-event-eventbridge-dataclass branch from cfb660f to 4a88320 Compare March 19, 2023 11:11
@pull-request-size pull-request-size bot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Mar 19, 2023
@ivica-k ivica-k force-pushed the feat(event-source)-1656-s3-event-eventbridge-dataclass branch from 4a88320 to 7e3614e Compare March 19, 2023 17:22
@heitorlessa heitorlessa self-assigned this Mar 20, 2023
@heitorlessa
Copy link
Contributor

Pulling this now ;)

@heitorlessa
Copy link
Contributor

I can't understand for the life of me why GH Actions is still picking up an old commit. Could you create a new one? Never seen this before.

@ivica-k
Copy link
Contributor Author

ivica-k commented Mar 20, 2023

Closing this for now - will create a fresh PR

@ivica-k ivica-k closed this Mar 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation feature New feature or functionality size/L Denotes a PR that changes 100-499 lines, ignoring generated files. tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Feature request: Support S3 Event Notifications Model for Event Bridge
2 participants