-
Notifications
You must be signed in to change notification settings - Fork 1
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
🐛 Fix: implementation replacement on redirect #176
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
❗ Your organization needs to install the Codecov GitHub app to enable full functionality. Additional details and impacted files@@ Coverage Diff @@
## dev #176 +/- ##
=======================================
Coverage 79.85% 79.85%
=======================================
Files 61 61
Lines 2050 2050
=======================================
Hits 1637 1637
Misses 413 413
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚨 Try these New Features:
|
82042cd
to
27ef81a
Compare
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.
Looks good! Thanks for doing this.
...orm/modules/content_delivery_network/lambda_functions/redirect_s3_404/src/lambda_function.py
Show resolved
Hide resolved
06b1922
to
590705e
Compare
...orm/modules/content_delivery_network/lambda_functions/redirect_s3_404/src/lambda_function.py
Show resolved
Hide resolved
* 🐛 fix: Only replace implementation with `dynamic` Supports FLAG-1126 * ✅ test: add initial pytest suite * 🎨 refactor: create more declarative methods and tests * 📝 docs: add docstring to handler * 📝 docs: add implementation note regarding relative URLs (cherry picked from commit 8d79ee7)
* 🐛 fix: Only replace implementation with `dynamic` Supports FLAG-1126 * ✅ test: add initial pytest suite * 🎨 refactor: create more declarative methods and tests * 📝 docs: add docstring to handler * 📝 docs: add implementation note regarding relative URLs (cherry picked from commit 8d79ee7)
* 🐛 fix: Only replace implementation with `dynamic` Supports FLAG-1126 * ✅ test: add initial pytest suite * 🎨 refactor: create more declarative methods and tests * 📝 docs: add docstring to handler * 📝 docs: add implementation note regarding relative URLs (cherry picked from commit 8d79ee7) (cherry picked from commit bc6a6c7)
* 🐛 fix: Only replace implementation with `dynamic` Supports FLAG-1126 * ✅ test: add initial pytest suite * 🎨 refactor: create more declarative methods and tests * 📝 docs: add docstring to handler * 📝 docs: add implementation note regarding relative URLs (cherry picked from commit 8d79ee7) (cherry picked from commit bc6a6c7)
Pull request checklist
Please check if your PR fulfills the following requirements:
Pull request type
Please check the type of change your PR introduces:
What is the current behavior?
The string literal
dynamic
replaces any part of the URL path that matches theimplementation
part of the URL.For example:
/sbtn_
natural_forest
s_map/v202310/natural_forest
/10/20/30.pngwas resulting in the following incorrect redirect URL:
/sbtn_
dynamic
s_map/v202310/dynamic
/10/20/30.png?implementation=natural_forest
What is the new behavior?
Now, the URL:
/sbtn_natural_forests_map/v202310/natural_forest/10/20/30.png
Is correctly transformed into:
/sbtn_natural_forests_map/v202310/dynamic/10/20/30.png?implementation=natural_forest
Does this introduce a breaking change?