-
Notifications
You must be signed in to change notification settings - Fork 401
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): Extend CodePipeline Artifact Capabilities #5448
feat(event_source): Extend CodePipeline Artifact Capabilities #5448
Conversation
aws_lambda_powertools/utilities/data_classes/code_pipeline_job_event.py
Outdated
Show resolved
Hide resolved
7e8f082
to
9eb8ba9
Compare
98bac5d
to
0e04534
Compare
Regarding the above issues from SonarCloud: These are all mocked methods of the S3 client and don't match the |
0e04534
to
941733c
Compare
Hi @mw-root! Thanks for sending this PR! This week is super busy, but I'll start reviewing it today, ok? |
@leandrodamascena glad to do it and no rush at all, thanks for taking it on! |
53ca7f6
to
b092fa7
Compare
b092fa7
to
f68bafe
Compare
Hey @mw-root ! I'll be helping @leandrodamascena to review it. Thank you so much for the PR! Great content |
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.
My initial tests appear to be successful. @leandrodamascena, could you please conduct an additional review of the user experience before we proceed?
Hi @mw-root , just ran our CI here, and mypy is complaining about the None type here. I saw that Item "None" of "CodePipelineEncryptionKey | None" has no attribute "get_id"
[union-attr]
encryption_key_id = self.data.encryption_key.get_id
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
aws_lambda_powertools/utilities/data_classes/code_pipeline_job_event.py:313:31: error:
Item "None" of "CodePipelineEncryptionKey | None" has no attribute "get_type"
[union-attr]
encryption_key_type = self.data.encryption_key.get_type
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ Thanks! Let me know if you need help with this |
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 @mw-root! Super nice PR! I just left a comment and please try to address it.
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! The PR is looking great, can you please take a look in the mypy error? Thanks!
Sorry about that, commited a fix for the mypy issues. Thanks! |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #5448 +/- ##
===========================================
- Coverage 96.16% 96.16% -0.01%
===========================================
Files 229 229
Lines 10815 10836 +21
Branches 2009 2015 +6
===========================================
+ Hits 10400 10420 +20
Misses 327 327
- Partials 88 89 +1 ☔ View full report in Codecov by Sentry. |
I'll deal with the coverage gap later today. |
Awesome, thank you! |
I've pushed a commit that should take care of codecov. |
Hi @anafalcao, just checking in.. I think I caught everything up.. can you confirm? |
Hey @mw-root , sorry for the late reply, it was a long weekend in my country. I'm back today and I'll review it now. Thank you very much for your proactivity in solving this quickly |
Quality Gate passedIssues Measures |
No need to apologize @anafalcao, thanks again and I hope you had a good long weekend! |
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.
Thank you for addressing all the feedback! You've made great improvements to the code, tests and doc. Well done!!
Issue number: #5447
Summary
Changes
This change adds the ability to upload output artifacts in a simple way, pulling all possible config from the event itself.
User experience
Checklist
If your change doesn't seem to apply, please leave them unchecked.
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.