Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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: add expand_slashed_path_patterns flag #4813
base: main
Are you sure you want to change the base?
feat: add expand_slashed_path_patterns flag #4813
Changes from 5 commits
a1f0871
74da0f5
692d9f9
fc2b47b
c1f25b6
526aee9
745b0f2
9a7d834
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
I just realized that we have a package for parsing these paths already: https:/grpc-ecosystem/grpc-gateway/blob/main/internal/httprule. What do you think about reusing this?
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.
I tried that package but unfortunately, it doesn't fit as a replacement for the
templateToParts
function, I encountered these problemstemplateToParts
split the template by slashes, so a template with a leading slash creates an empty string at the start of the output slice, trailing slash in the template creates an empty string at the end of the slice - I can go around this, buthttprule.Parse
do not even accept templates without trailing slash, which broke some tests - I guess that all templates should generally be with trailing slash, but I'm not certain.httprule.Parse
ignores and omits the path patterns, these are completely lost in theCompile
output object, this was a blocker,Compile.Pool
contains template segments, but the order is not guaranteed, I encountered a test with a path param where the order of the segments was different than in the original template, this is hard to resolve,Overall,
httprule.Parse
does a lot of stuff, but most of it is not useful for thetemplateToParts
, and the important stuff is missing and hard to add. At least, I managed to simplify thetemplateToParts
and fix the camel-casing, which wrongly affected the parameter pattern.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.
Thanks for the explanation, that makes sense!