Skip to content

fix(parser): S3Model Object Deleted omits size and eTag attr #1638

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Merged
merged 2 commits into from
Oct 24, 2022

Conversation

barreeeiroo
Copy link
Contributor

@barreeeiroo barreeeiroo commented Oct 23, 2022

Issue number: #1637

Summary

Fixes S3 ObjectRemoved API event, as they don't include some attributes attributes.

Changes

  • Flags eTag as optional attribute in S3Model.
  • Flags size as optional attribute in S3Model.

Checklist

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

Is this a breaking change?

No.

Testing

Thanks @ran-isenberg for the unit tests! They are all passing:

Testing started at 21:33 ...
Launching pytest with arguments test_s3.py --no-header --no-summary -q in C:\Github\aws-lambda-powertools-python\tests\functional\parser

============================= test session starts =============================
collecting ... collected 6 items

test_s3.py::test_s3_trigger_event PASSED                                 [ 16%]
test_s3.py::test_s3_glacier_trigger_event PASSED                         [ 33%]
test_s3.py::test_s3_empty_object PASSED                                  [ 50%]
test_s3.py::test_s3_none_object_size_failed_validation PASSED            [ 66%]
test_s3.py::test_s3_none_etag_value_failed_validation PASSED             [ 83%]
test_s3.py::test_s3_trigger_event_delete_object PASSED                   [100%]

============================== 6 passed in 0.09s ==============================

Process finished with exit code 0

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.

@barreeeiroo barreeeiroo requested a review from a team as a code owner October 23, 2022 18:06
@barreeeiroo barreeeiroo requested review from heitorlessa and removed request for a team October 23, 2022 18:06
@boring-cyborg
Copy link

boring-cyborg bot commented Oct 23, 2022

Thanks a lot for your first contribution! Please check out our contributing guidelines and don't hesitate to ask whatever you need.
In the meantime, check out the #python channel on our AWS Lambda Powertools Discord: Invite link

@pull-request-size pull-request-size bot added the size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. label Oct 23, 2022
@barreeeiroo barreeeiroo changed the title fix(parser): Mark size and eTag as optional attributes fix(parser): mark size and eTag as optional attributes in S3Model Oct 23, 2022
barreeeiroo and others added 2 commits October 23, 2022 21:31
Fixes ObjectRemoved API event, as they don't include these attributes.
aws-powertools#1637

Signed-off-by: Diego Barreiro <diego.barreiro.perez@gmail.com>
@boring-cyborg boring-cyborg bot added the tests label Oct 23, 2022
@pull-request-size pull-request-size bot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Oct 23, 2022
@ran-isenberg
Copy link
Contributor

LGTM

@github-actions github-actions bot added the bug Something isn't working label Oct 24, 2022
@codecov-commenter
Copy link

codecov-commenter commented Oct 24, 2022

Codecov Report

Base: 99.76% // Head: 99.35% // Decreases project coverage by -0.40% ⚠️

Coverage data is based on head (8bfab74) compared to base (416ab1b).
Patch coverage: 87.56% of modified lines in pull request are covered.

Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #1638      +/-   ##
===========================================
- Coverage    99.76%   99.35%   -0.41%     
===========================================
  Files          127      128       +1     
  Lines         5884     5881       -3     
  Branches       668      373     -295     
===========================================
- Hits          5870     5843      -27     
- Misses           6       18      +12     
- Partials         8       20      +12     
Impacted Files Coverage Δ
aws_lambda_powertools/shared/constants.py 100.00% <ø> (ø)
aws_lambda_powertools/utilities/batch/__init__.py 100.00% <ø> (ø)
aws_lambda_powertools/utilities/batch/base.py 97.79% <ø> (-1.48%) ⬇️
...ws_lambda_powertools/utilities/batch/exceptions.py 100.00% <ø> (ø)
aws_lambda_powertools/shared/cookies.py 58.69% <58.69%> (ø)
...bda_powertools/utilities/data_classes/alb_event.py 90.47% <60.00%> (-9.53%) ⬇️
...ambda_powertools/utilities/parameters/appconfig.py 94.28% <86.66%> (-5.72%) ⬇️
...s/utilities/data_classes/dynamo_db_stream_event.py 98.13% <95.45%> (-1.87%) ⬇️
aws_lambda_powertools/event_handler/api_gateway.py 100.00% <100.00%> (ø)
aws_lambda_powertools/shared/headers_serializer.py 100.00% <100.00%> (ø)
... and 8 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@heitorlessa heitorlessa changed the title fix(parser): mark size and eTag as optional attributes in S3Model fix(parser): S3Model Object Deleted omits size and eTag attr Oct 24, 2022
@heitorlessa heitorlessa merged commit 1eaa85d into aws-powertools:develop Oct 24, 2022
@boring-cyborg
Copy link

boring-cyborg bot commented Oct 24, 2022

Awesome work, congrats on your first merged pull request and thank you for helping improve everyone's experience!

@barreeeiroo barreeeiroo deleted the patch-1 branch August 4, 2023 11:12
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
bug Something isn't working 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.

4 participants