-
Notifications
You must be signed in to change notification settings - Fork 421
feat(event_source): add CloudFormationCustomResourceEvent data class. #4342
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
feat(event_source): add CloudFormationCustomResourceEvent data class. #4342
Conversation
Thanks a lot for your first contribution! Please check out our contributing guidelines and don't hesitate to ask whatever you need. |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #4342 +/- ##
===========================================
- Coverage 96.38% 96.32% -0.06%
===========================================
Files 214 219 +5
Lines 10030 10485 +455
Branches 1846 1941 +95
===========================================
+ Hits 9667 10100 +433
- Misses 259 271 +12
- Partials 104 114 +10 ☔ View full report in Codecov by Sentry. |
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.
Hey @phipag! Super nice this PR! I just left some few comments to make it even better.
Please let me know if you have any question.
aws_lambda_powertools/utilities/data_classes/cloudformation_custom_resource_event.py
Outdated
Show resolved
Hide resolved
aws_lambda_powertools/utilities/data_classes/cloudformation_custom_resource_event.py
Outdated
Show resolved
Hide resolved
aws_lambda_powertools/utilities/data_classes/cloudformation_custom_resource_event.py
Outdated
Show resolved
Hide resolved
aws_lambda_powertools/utilities/data_classes/cloudformation_custom_resource_event.py
Outdated
Show resolved
Hide resolved
aws_lambda_powertools/utilities/data_classes/cloudformation_custom_resource_event.py
Outdated
Show resolved
Hide resolved
aws_lambda_powertools/utilities/data_classes/cloudformation_custom_resource_event.py
Outdated
Show resolved
Hide resolved
aws_lambda_powertools/utilities/data_classes/cloudformation_custom_resource_event.py
Outdated
Show resolved
Hide resolved
aws_lambda_powertools/utilities/parser/models/cloudformation_custom_resource.py
Show resolved
Hide resolved
aws_lambda_powertools/utilities/parser/models/cloudformation_custom_resource.py
Show resolved
Hide resolved
Thanks for your review. All makes sense. I’ll send an update on Tuesday. 👍🏻 |
I addressed your comments @leandrodamascena and pushed a new commit. |
Super thanks @phipag! I see some errors in CI, but those are not related with this PR and I'm fixing this. I'm going to review this PR today/tomorrow, ok? |
Sure. No problem. Should I merge from develop next time before sending an update? |
Hmm, you don't need this unless we see some conflict that blocks the merge. But in this case the problem is because a PR was approved this morning and the linter is complaining. Basically it was my fault because I didn't run the linter locally #mybad 🫨 |
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.
Hey @phipag! We need to make a very small change before we merge. 🚀
Good catch. I'll fix it tomorrow morning! 👍🏻 |
|
Hey @leandrodamascena I pushed the update. |
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.
Hey @phipag! Thank you for your first contribution to the project. The code is clean and working perfectly for what was proposed. Feel free to contribute as many times as you like! 🚀
54e6cdd
into
aws-powertools:develop
Awesome! 🙌 |
Issue number: #4332
Summary
This adds the
CloudFormationCustomResourceEvent
event source data class. There was already a parser but the event source data class was missing.Changes
CloudFormationCustomResourceEvent
data classPhysicalResourceId
to parserUser experience
As a user I was unable to add a strongly typed event to my Lambda handler for custom CloudFormation resource management.
After the change, I am able to have a typed event for this scenario.
Checklist
If your change doesn't seem to apply, please leave them unchecked.
Is this a breaking change? No
RFC issue number:
Checklist:
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.