-
Notifications
You must be signed in to change notification settings - Fork 402
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(events): Update and Add Cognito User Pool Events #4423
feat(events): Update and Add Cognito User Pool Events #4423
Conversation
Thanks a lot for your first contribution! Please check out our contributing guidelines and don't hesitate to ask whatever you need. |
Hi @knightmre this is a super detailed PR with an impressive level of awesome docs/documentation. I'm working on other PRs and will schedule the first review of this PR for early next week. Thanks |
Hi, thanks for your time, I just found out I missed out on some tests so I'll be reworking this a little before your review. |
f64d099
to
9002964
Compare
a6e4c77
to
4bba674
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## develop #4423 +/- ##
===========================================
+ Coverage 96.38% 96.43% +0.05%
===========================================
Files 214 219 +5
Lines 10030 10623 +593
Branches 1846 1976 +130
===========================================
+ Hits 9667 10244 +577
- Misses 259 267 +8
- Partials 104 112 +8 ☔ View full report in Codecov by Sentry. |
4bba674
to
1129ce9
Compare
Reviewing this now. |
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 @knightmre! Well done! This was one of the most detailed PRs I've looked at. Creating all those events in Cognito must have taken a lot of hard work and I'm sure you spent some days. I tried making some myself, and it took me over a day.
I just made a few small comments to your great work. 🚀
aws_lambda_powertools/utilities/data_classes/cognito_user_pool_event.py
Outdated
Show resolved
Hide resolved
aws_lambda_powertools/utilities/data_classes/cognito_user_pool_event.py
Outdated
Show resolved
Hide resolved
aws_lambda_powertools/utilities/data_classes/cognito_user_pool_event.py
Outdated
Show resolved
Hide resolved
aws_lambda_powertools/utilities/data_classes/cognito_user_pool_event.py
Outdated
Show resolved
Hide resolved
aws_lambda_powertools/utilities/data_classes/cognito_user_pool_event.py
Outdated
Show resolved
Hide resolved
aws_lambda_powertools/utilities/data_classes/cognito_user_pool_event.py
Outdated
Show resolved
Hide resolved
Quality Gate passedIssues Measures |
1b7c873
to
036b3e0
Compare
Hi @leandrodamascena Thanks for the review. I've included your comments and updated the PR. |
Quality Gate passedIssues Measures |
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.
I'm going to say this again: amazing work @knightmre!
APPROVED!
Awesome work, congrats on your first merged pull request and thank you for helping improve everyone's experience! |
Issue number: #3987
Summary
Changes
This change updates the UserMigrationTriggerEvent data class, adds the V2 of Pre Token Generation Lambda Triggers and adds two new data classes for Custom Sender Lambda triggers
User experience
Events did not exist before. A workaround was to self-implement the data classes by inheriting base classes like so:
Before
After
Checklist
If your change doesn't seem to apply, please leave them unchecked.
Is this a breaking change?
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.