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

feat(event_source): Extend CodePipeline Artifact Capabilities #5448

Conversation

mw-root
Copy link
Contributor

@mw-root mw-root commented Oct 24, 2024

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

import json 

from aws_lambda_powertools import Logger
from aws_lambda_powertools.utilities.data_classes import event_source, CodePipelineJobEvent

logger = Logger()

@event_source(data_class=CodePipelineJobEvent)
def lambda_handler(event: CodePipelineJobEvent, context):
    ...
    output_artifact = do_the_thing()
    event.put_artifact(
        artifact_name="json-artifact",
        body=json.dumps(output_artifact),
        content_type="application/json"
    )

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.

@boring-cyborg boring-cyborg bot added documentation Improvements or additions to documentation tests labels Oct 24, 2024
@pull-request-size pull-request-size bot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Oct 24, 2024
@mw-root mw-root force-pushed the expand-codepipeline-artifact-capabilities branch from 7e8f082 to 9eb8ba9 Compare October 24, 2024 17:43
@mw-root mw-root marked this pull request as ready for review October 24, 2024 17:44
@mw-root mw-root requested a review from a team as a code owner October 24, 2024 17:44
@mw-root mw-root force-pushed the expand-codepipeline-artifact-capabilities branch 2 times, most recently from 98bac5d to 0e04534 Compare October 25, 2024 15:18
@mw-root
Copy link
Contributor Author

mw-root commented Oct 29, 2024

Regarding the above issues from SonarCloud:

These are all mocked methods of the S3 client and don't match the snake_case convention because boto3 doesn't ;)

@mw-root mw-root force-pushed the expand-codepipeline-artifact-capabilities branch from 0e04534 to 941733c Compare October 29, 2024 13:07
@leandrodamascena
Copy link
Contributor

Hi @mw-root! Thanks for sending this PR! This week is super busy, but I'll start reviewing it today, ok?

@mw-root
Copy link
Contributor Author

mw-root commented Oct 29, 2024

@leandrodamascena glad to do it and no rush at all, thanks for taking it on!

@mw-root mw-root force-pushed the expand-codepipeline-artifact-capabilities branch 2 times, most recently from 53ca7f6 to b092fa7 Compare October 31, 2024 17:51
@mw-root mw-root force-pushed the expand-codepipeline-artifact-capabilities branch from b092fa7 to f68bafe Compare November 5, 2024 20:45
@anafalcao
Copy link
Collaborator

Hey @mw-root ! I'll be helping @leandrodamascena to review it. Thank you so much for the PR! Great content

Copy link
Collaborator

@anafalcao anafalcao left a 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?

docs/utilities/data_classes.md Outdated Show resolved Hide resolved
@anafalcao
Copy link
Collaborator

Hi @mw-root , just ran our CI here, and mypy is complaining about the None type here. I saw that self.data.encryption_key can be CodePipelineEncryptionKey or None.
Can you please take a look?

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

Copy link
Contributor

@leandrodamascena leandrodamascena left a 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.

Copy link
Collaborator

@anafalcao anafalcao left a 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!

@mw-root
Copy link
Contributor Author

mw-root commented Nov 14, 2024

@anafalcao

Sorry about that, commited a fix for the mypy issues.

Thanks!

Copy link

codecov bot commented Nov 14, 2024

Codecov Report

Attention: Patch coverage is 96.55172% with 1 line in your changes missing coverage. Please review.

Project coverage is 96.16%. Comparing base (9991702) to head (a7831d3).
Report is 2 commits behind head on develop.

Files with missing lines Patch % Lines
.../utilities/data_classes/code_pipeline_job_event.py 96.55% 0 Missing and 1 partial ⚠️
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.
📢 Have feedback on the report? Share it here.

@mw-root
Copy link
Contributor Author

mw-root commented Nov 14, 2024

I'll deal with the coverage gap later today.

@anafalcao
Copy link
Collaborator

I'll deal with the coverage gap later today.

Awesome, thank you!

@mw-root mw-root requested a review from anafalcao November 14, 2024 18:17
@mw-root
Copy link
Contributor Author

mw-root commented Nov 14, 2024

I've pushed a commit that should take care of codecov.

@mw-root
Copy link
Contributor Author

mw-root commented Nov 15, 2024

Hi @anafalcao, just checking in.. I think I caught everything up.. can you confirm?

@anafalcao
Copy link
Collaborator

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

@mw-root
Copy link
Contributor Author

mw-root commented Nov 18, 2024

No need to apologize @anafalcao, thanks again and I hope you had a good long weekend!

Copy link
Collaborator

@anafalcao anafalcao left a 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!!

@anafalcao anafalcao merged commit b99884e into aws-powertools:develop Nov 18, 2024
12 checks passed
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 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: Add Functionality to allow CodePipeline Event to Manage Output Artifacts
3 participants