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(parser): add support for Lambda Function URL #1442

Merged
merged 5 commits into from
Aug 18, 2022

Conversation

ran-isenberg
Copy link
Contributor

@ran-isenberg ran-isenberg commented Aug 11, 2022

Issue number: #1425

Summary

Changes

added envelope and model for lambda function url as described at #1425

fixed a bug in api gwv2 model where cognitoIdentity wasnt optional.

I was unable to fix the docs for the parser due to a linter bug, will open an issue.

User experience

Can now use an envelope for lambda function url or extend the model.

Is this a breaking change? No.

RFC issue number:
#1425
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.

@ran-isenberg ran-isenberg requested a review from a team as a code owner August 11, 2022 16:14
@ran-isenberg ran-isenberg requested review from mploski and removed request for a team August 11, 2022 16:14
@pull-request-size pull-request-size bot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Aug 11, 2022
@ran-isenberg
Copy link
Contributor Author

@heitorlessa i wasnt able to push the docs update.
They pass the docs-linter stage but fail on the pre-commit on this error:
docs/utilities/parser.md:586 MD043/required-headings/required-headers Required heading structure [Context: "## Getting started"]

@leandrodamascena
Copy link
Contributor

leandrodamascena commented Aug 11, 2022

Hi @ran-isenberg.. I'm refactoring all examples and for now you can add this comment after the description line..

<!-- markdownlint-disable MD043 -->

like this: 7be425e#diff-aa083264efcaf3125cc0560e9b88cce2c69e4becd8ea39211057634ec3f56bd9R6

@heitorlessa heitorlessa changed the title feat: Add Parser official support for Lambda Function UR feat(parser): add support for Lambda Function URL Aug 11, 2022
@github-actions github-actions bot added the feature New feature or functionality label Aug 11, 2022
@heitorlessa
Copy link
Contributor

Thanks a lot @ran-isenberg !! As @leandrodamascena shared, this is the new doc linting to enforce a consistent style, you can ignore for now with that string at the top of the file since we're refactoring all of it (structure, snippets to be way more realistic, run as-is).

@leandrodamascena could you please fix that mypy issue by installing snappy as a dev dependency and ignoring the lack of types for snappy? We missed in the last PR as I accidentally didn't allow GH Actions to run (part of the new repo hardening)

@boring-cyborg boring-cyborg bot added the documentation Improvements or additions to documentation label Aug 11, 2022
@ran-isenberg
Copy link
Contributor Author

pushed the docs ;)

@ran-isenberg
Copy link
Contributor Author

@heitorlessa it failed on the snappy stuff. I updated from develop branch, hopefully it will work now

@heitorlessa
Copy link
Contributor

heitorlessa commented Aug 12, 2022 via email

@codecov-commenter
Copy link

codecov-commenter commented Aug 12, 2022

Codecov Report

Merging #1442 (ddf1dd1) into develop (0e20ee0) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff            @@
##           develop    #1442   +/-   ##
========================================
  Coverage    99.89%   99.89%           
========================================
  Files          121      123    +2     
  Lines         5479     5496   +17     
  Branches       627      629    +2     
========================================
+ Hits          5473     5490   +17     
  Misses           2        2           
  Partials         4        4           
Impacted Files Coverage Δ
..._powertools/utilities/parser/envelopes/__init__.py 100.00% <100.00%> (ø)
.../utilities/parser/envelopes/lambda_function_url.py 100.00% <100.00%> (ø)
...bda_powertools/utilities/parser/models/__init__.py 100.00% <100.00%> (ø)
...mbda_powertools/utilities/parser/models/apigwv2.py 100.00% <100.00%> (ø)
...ols/utilities/parser/models/lambda_function_url.py 100.00% <100.00%> (ø)

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

@heitorlessa
Copy link
Contributor

@rubenfonseca assigned it to you as you fixed the quirks of Function URL.

@ran-isenberg CI passed. Ruben is back on Monday from holidays and should get this reviewed and merged next week.

Thank you again

@rubenfonseca
Copy link
Contributor

Just rebased with master before the review

Copy link
Contributor

@rubenfonseca rubenfonseca left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @ran-isenberg thank you for this! Just left two small suggestions. Let me know if it makes sense so we can merge.

docs/utilities/parser.md Outdated Show resolved Hide resolved
ran-isenberg and others added 2 commits August 16, 2022 17:03
…on_url.py

Co-authored-by: Ruben Fonseca <fonseka@gmail.com>
Co-authored-by: Ruben Fonseca <fonseka@gmail.com>
@rubenfonseca rubenfonseca self-requested a review August 16, 2022 14:41
@leandrodamascena
Copy link
Contributor

Hi @ran-isenberg thank you for this! Just left two small suggestions. Let me know if it makes sense so we can merge.

Hi!! Just in case of better understanding of the project I did a double check and everything seems fine to merge. I ran these changes locally and it went fine.

@ran-isenberg
Copy link
Contributor Author

@heitorlessa waiting for the merge :)

@heitorlessa
Copy link
Contributor

@rubenfonseca will be merging ;) Thanks a lot everyone!!

@rubenfonseca rubenfonseca merged commit 1d7a4ab into aws-powertools:develop Aug 18, 2022
@rubenfonseca
Copy link
Contributor

Thanks again Ran! Can't wait to get this out soon :)

@ran-isenberg ran-isenberg deleted the func_url branch September 4, 2022 05:22
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 feature New feature or functionality 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.

5 participants