feat(event_handler): allow stripping route prefixes using regexes #2521

royassis · 2023-06-22T13:37:44Z

Issue number: #2494

Summary

Changes

This PR adds the ability to set a regex when removing prefixes from routes. This is in addition to the existing mechanism where we strip static strings as prefixes.

User experience

Please share what the user experience looks like before and after this change

Checklist

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

Meet tenets criteria
I have performed a self-review of this change
Changes have been tested
Changes are documented
PR title follows conventional commit semantics

Is this a breaking change?

RFC issue number:

Checklist:

Migration process documented
Implement warnings (if it can live side by side)

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 · 2023-06-22T13:37:47Z

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 Powertools for AWS Lambda Discord: Invite link

github-actions · 2023-06-27T13:14:39Z

No related issues found. Please ensure there is an open issue related to this change to avoid significant delays or closure.

rubenfonseca · 2023-06-27T13:34:05Z

Looking into this now

rubenfonseca

Thank you for your contribution! 🎉 Two asks:

can you make this non-iterative? Meaning, when there is a match in the regex, we stop processing more items from the strip_prefixes array? Reason: This would probably be a breaking change and using the regex solves the initial problem in the first place
do you mind adding a couple of functional test cases to cover the new code?

Please let me know if you need assistance with those, and we would be happy to help!

codecov-commenter · 2023-06-28T09:34:37Z

Codecov Report

Patch coverage: 91.86% and project coverage change: +0.10% 🎉

Comparison is base (b0a3658) 96.34% compared to head (43c1618) 96.44%.
Report is 9 commits behind head on develop.

❗ Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more.

Additional details and impacted files

@@             Coverage Diff             @@
##           develop    #2521      +/-   ##
===========================================
+ Coverage    96.34%   96.44%   +0.10%     
===========================================
  Files          169      171       +2     
  Lines         7678     7690      +12     
  Branches      1451     1455       +4     
===========================================
+ Hits          7397     7417      +20     
+ Misses         222      218       -4     
+ Partials        59       55       -4

Files Changed	Coverage Δ
...s_lambda_powertools/metrics/provider/cold_start.py	`100.00% <ø> (ø)`
aws_lambda_powertools/metrics/functions.py	`73.68% <73.68%> (ø)`
aws_lambda_powertools/metrics/provider/base.py	`95.23% <88.23%> (-2.06%)`	⬇️
aws_lambda_powertools/event_handler/api_gateway.py	`100.00% <100.00%> (ø)`
...da_powertools/event_handler/lambda_function_url.py	`100.00% <100.00%> (ø)`
aws_lambda_powertools/event_handler/vpc_lattice.py	`100.00% <100.00%> (ø)`
aws_lambda_powertools/metrics/base.py	`71.23% <100.00%> (ø)`
aws_lambda_powertools/metrics/metrics.py	`96.36% <100.00%> (+4.69%)`	⬆️
aws_lambda_powertools/metrics/provider/__init__.py	`100.00% <100.00%> (ø)`
...ools/metrics/provider/cloudwatch_emf/cloudwatch.py	`94.44% <100.00%> (+2.59%)`	⬆️
... and 1 more

... and 1 file with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

leandrodamascena · 2023-08-01T14:11:47Z

Thank you for your contribution! 🎉 Two asks:

can you make this non-iterative? Meaning, when there is a match in the regex, we stop processing more items from the strip_prefixes array? Reason: This would probably be a breaking change and using the regex solves the initial problem in the first place

do you mind adding a couple of functional test cases to cover the new code?

Please let me know if you need assistance with those, and we would be happy to help!

I asked for your review @rubenfonseca again! Since we haven't heard from other clients, we need to decide whether to go ahead with this PR and close it.

Thanks

heitorlessa · 2023-08-09T08:16:28Z

Hey! We'll be making the necessary adjustments to the PR to guarantee it solves your problem and of similar customers.

rubenfonseca · 2023-08-10T12:22:45Z

Looking into this now

leandrodamascena · 2023-08-10T23:30:43Z

Hei @heitorlessa, can you review this, please?

heitorlessa

Looks great, specially docs!

Asked for an additional test (if I read it correctly), and one question on whether we should warn

aws_lambda_powertools/event_handler/api_gateway.py

docs/core/event_handler/api_gateway.md

tests/functional/event_handler/test_api_gateway.py

heitorlessa

thank you!! this looks great! 🚀

boring-cyborg · 2023-08-14T09:04:11Z

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

sonarqubecloud · 2023-08-14T09:04:16Z

Kudos, SonarCloud Quality Gate passed!

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

No Coverage information
0.2% Duplication

…s-powertools#2521) Co-authored-by: Roy Assis <[email protected]> Co-authored-by: Ruben Fonseca <[email protected]>

* develop: chore: cleanup, add test for single and nested fix(parameters): make cache aware of single vs multiple calls docs(roadmap): add GovCloud and China region item (aws-powertools#2960) docs(metrics): update Datadog integration diagram (aws-powertools#2954) chore(ci): changelog rebuild (aws-powertools#2958) chore(deps-dev): bump cfn-lint from 0.79.6 to 0.79.7 (aws-powertools#2956) chore(deps): bump actions/setup-node from 3.7.0 to 3.8.0 (aws-powertools#2957) chore(deps-dev): bump xenon from 0.9.0 to 0.9.1 (aws-powertools#2955) feat(metrics): add Datadog observability provider (aws-powertools#2906) feat(event_handler): allow stripping route prefixes using regexes (aws-powertools#2521) chore(ci): changelog rebuild (aws-powertools#2952) chore(deps): bump pypa/gh-action-pypi-publish from 1.8.9 to 1.8.10 (aws-powertools#2946) chore(deps): bump gitpython from 3.1.31 to 3.1.32 in /docs (aws-powertools#2948) chore(deps-dev): bump aws-cdk from 2.90.0 to 2.91.0 (aws-powertools#2947) chore(ci): changelog rebuild (aws-powertools#2945) chore(deps-dev): bump the boto-typing group with 1 update (aws-powertools#2944) chore(deps): bump pypa/gh-action-pypi-publish from 1.8.8 to 1.8.9 (aws-powertools#2943) Signed-off-by: heitorlessa <[email protected]>

* fix(parameters): make cache aware of single vs multiple calls Signed-off-by: heitorlessa <[email protected]> * chore: cleanup, add test for single and nested Signed-off-by: heitorlessa <[email protected]> * chore(deps): bump pypa/gh-action-pypi-publish from 1.8.8 to 1.8.9 (aws-powertools#2943) Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * chore(deps-dev): bump the boto-typing group with 1 update (aws-powertools#2944) Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Leandro Damascena <[email protected]> * chore(ci): changelog rebuild (aws-powertools#2945) Co-authored-by: Powertools for AWS Lambda (Python) bot <[email protected]> * chore(deps-dev): bump aws-cdk from 2.90.0 to 2.91.0 (aws-powertools#2947) Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * chore(deps): bump gitpython from 3.1.31 to 3.1.32 in /docs (aws-powertools#2948) Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * chore(deps): bump pypa/gh-action-pypi-publish from 1.8.9 to 1.8.10 (aws-powertools#2946) Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Leandro Damascena <[email protected]> * chore(ci): changelog rebuild (aws-powertools#2952) Co-authored-by: Powertools for AWS Lambda (Python) bot <[email protected]> * feat(event_handler): allow stripping route prefixes using regexes (aws-powertools#2521) Co-authored-by: Roy Assis <[email protected]> Co-authored-by: Ruben Fonseca <[email protected]> * feat(metrics): add Datadog observability provider (aws-powertools#2906) Co-authored-by: Leandro Damascena <[email protected]> Co-authored-by: heitorlessa <[email protected]> * chore(deps-dev): bump xenon from 0.9.0 to 0.9.1 (aws-powertools#2955) Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * chore(deps): bump actions/setup-node from 3.7.0 to 3.8.0 (aws-powertools#2957) Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * chore(deps-dev): bump cfn-lint from 0.79.6 to 0.79.7 (aws-powertools#2956) Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Leandro Damascena <[email protected]> * chore(ci): changelog rebuild (aws-powertools#2958) Co-authored-by: Powertools for AWS Lambda (Python) bot <[email protected]> * docs(metrics): update Datadog integration diagram (aws-powertools#2954) Co-authored-by: Leandro Damascena <[email protected]> * docs(roadmap): add GovCloud and China region item (aws-powertools#2960) * fix(parameters): make cache aware of single vs multiple calls Signed-off-by: heitorlessa <[email protected]> * chore: cleanup, add test for single and nested Signed-off-by: heitorlessa <[email protected]> * chore(test): remove itsdangerous from perf test Signed-off-by: heitorlessa <[email protected]> * chore(deps): remove itsdangerous dependencies * chore: disable sockets in encryption sdk tests Signed-off-by: heitorlessa <[email protected]> * refactor(tests): use a test double * chore: address make pr errors Signed-off-by: heitorlessa <[email protected]> --------- Signed-off-by: heitorlessa <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Leandro Damascena <[email protected]> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: Powertools for AWS Lambda (Python) bot <[email protected]> Co-authored-by: roy <[email protected]> Co-authored-by: Roy Assis <[email protected]> Co-authored-by: Ruben Fonseca <[email protected]> Co-authored-by: Roger Zhang <[email protected]> Co-authored-by: aal80 <[email protected]> Co-authored-by: Seshu Brahma <[email protected]>

royassis requested a review from a team as a code owner June 22, 2023 13:37

boring-cyborg bot added the event_handlers label Jun 22, 2023

pull-request-size bot added the size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. label Jun 22, 2023

royassis mentioned this pull request Jun 22, 2023

Feature request: Better strip-prefixes for APIGatewayRestResolver #2494

Closed

2 tasks

royassis changed the title ~~Update _remove_prefix method to iterativly remove prefix by regex~~ feat - update remove_prefix method to iteratively remove prefix by regex Jun 23, 2023

leandrodamascena requested review from leandrodamascena and removed request for a team June 26, 2023 08:21

rubenfonseca linked an issue Jun 27, 2023 that may be closed by this pull request

Feature request: Better strip-prefixes for APIGatewayRestResolver #2494

Closed

2 tasks

rubenfonseca changed the title ~~feat - update remove_prefix method to iteratively remove prefix by regex~~ feat(event_handler): iteratively remove prefix by regex Jun 27, 2023

github-actions bot added do-not-merge need-issue PRs that are missing related issues labels Jun 27, 2023

rubenfonseca force-pushed the improv/event_handler_api_gateway_strip_prefixes branch from e52c51a to d4b759f Compare June 27, 2023 13:14

github-actions bot added the feature New feature or functionality label Jun 27, 2023

rubenfonseca removed do-not-merge need-issue PRs that are missing related issues labels Jun 27, 2023

rubenfonseca suggested changes Jun 27, 2023

View reviewed changes

rubenfonseca added the revisit Maintainer to provide update or revisit prioritization in the next cycle label Jun 27, 2023

rubenfonseca force-pushed the improv/event_handler_api_gateway_strip_prefixes branch from d4b759f to 2a7e2b6 Compare June 28, 2023 09:31

leandrodamascena requested a review from rubenfonseca August 1, 2023 14:09

rubenfonseca changed the title ~~feat(event_handler): iteratively remove prefix by regex~~ feat(event_handler): strip prefixes using regex Aug 10, 2023

rubenfonseca changed the title ~~feat(event_handler): strip prefixes using regex~~ feat(event_handler): strip route prefixes using regex Aug 10, 2023

rubenfonseca force-pushed the improv/event_handler_api_gateway_strip_prefixes branch from 373aada to 766f525 Compare August 10, 2023 12:25

boring-cyborg bot added the tests label Aug 10, 2023

boring-cyborg bot added the documentation Improvements or additions to documentation label Aug 10, 2023

rubenfonseca changed the title ~~feat(event_handler): strip route prefixes using regex~~ feat(event_handler): strip route prefixes using regexes Aug 10, 2023

rubenfonseca changed the title ~~feat(event_handler): strip route prefixes using regexes~~ feat(event_handler): allow stripping of route prefixes using regexes Aug 10, 2023

rubenfonseca requested review from heitorlessa and removed request for rubenfonseca August 10, 2023 14:19

rubenfonseca removed the revisit Maintainer to provide update or revisit prioritization in the next cycle label Aug 10, 2023

heitorlessa suggested changes Aug 11, 2023

View reviewed changes

pull-request-size bot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Aug 11, 2023

rubenfonseca requested a review from heitorlessa August 11, 2023 13:37

heitorlessa approved these changes Aug 11, 2023

View reviewed changes

heitorlessa changed the title ~~feat(event_handler): allow stripping of route prefixes using regexes~~ feat(event_handler): allow stripping route prefixes using regexes Aug 11, 2023

leandrodamascena approved these changes Aug 11, 2023

View reviewed changes

leandrodamascena requested a review from rubenfonseca August 11, 2023 20:42

leandrodamascena approved these changes Aug 11, 2023

View reviewed changes

Roy Assis and others added 6 commits August 14, 2023 11:03

Update _remove_prefix method to iterativly remove prefix by regex

6c38ef6

fix: streamlined regexes

411dc2b

fix: update APIGateway subclasses

49f299b

chore(docs): added documentation

276293b

chore: reword

e60d3b3

fix: added tests

5684ef1

rubenfonseca force-pushed the improv/event_handler_api_gateway_strip_prefixes branch from 05d70db to 5684ef1 Compare August 14, 2023 09:03

rubenfonseca approved these changes Aug 14, 2023

View reviewed changes

rubenfonseca merged commit 0485c8a into aws-powertools:develop Aug 14, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(event_handler): allow stripping route prefixes using regexes #2521

feat(event_handler): allow stripping route prefixes using regexes #2521

royassis commented Jun 22, 2023 •

edited by rubenfonseca

Loading

boring-cyborg bot commented Jun 22, 2023

github-actions bot commented Jun 27, 2023

rubenfonseca commented Jun 27, 2023

rubenfonseca left a comment

codecov-commenter commented Jun 28, 2023 •

edited

Loading

leandrodamascena commented Aug 1, 2023

heitorlessa commented Aug 9, 2023

rubenfonseca commented Aug 10, 2023

leandrodamascena commented Aug 10, 2023

heitorlessa left a comment

heitorlessa left a comment

boring-cyborg bot commented Aug 14, 2023

sonarqubecloud bot commented Aug 14, 2023

feat(event_handler): allow stripping route prefixes using regexes #2521

feat(event_handler): allow stripping route prefixes using regexes #2521

Conversation

royassis commented Jun 22, 2023 • edited by rubenfonseca Loading

Summary

Changes

User experience

Checklist

Acknowledgment

boring-cyborg bot commented Jun 22, 2023

github-actions bot commented Jun 27, 2023

rubenfonseca commented Jun 27, 2023

rubenfonseca left a comment

Choose a reason for hiding this comment

codecov-commenter commented Jun 28, 2023 • edited Loading

Codecov Report

leandrodamascena commented Aug 1, 2023

heitorlessa commented Aug 9, 2023

rubenfonseca commented Aug 10, 2023

leandrodamascena commented Aug 10, 2023

heitorlessa left a comment

Choose a reason for hiding this comment

heitorlessa left a comment

Choose a reason for hiding this comment

boring-cyborg bot commented Aug 14, 2023

sonarqubecloud bot commented Aug 14, 2023

royassis commented Jun 22, 2023 •

edited by rubenfonseca

Loading

codecov-commenter commented Jun 28, 2023 •

edited

Loading