-
Notifications
You must be signed in to change notification settings - Fork 2.2k
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
Overriding path parameter names not helpful with additional_bindings #2642
Comments
I'm not entirely sure I follow, but does option 2 imply parsing the path parameters and then constructing a regex that will match all of them? CC @oyvindwe |
I see the problem - we got the same problem in one of our own methods. It is also confusing because the parameter documentation is shared. Merging the paths and the regexes would be a very nice solution. Merging the regexes can probably simply be done by or-ing all the individual patterns, e.g. Note that this only applies to additional bindings that share paths, e.g. regexes should not be merged for cases like this:
|
@yinzara Btw, we use readme.com for API browsing, which does show However, as pointed out here #720 (comment) I suggest closing this issue and revisit when (if) OpenAPI supports |
I only see that as a "nice to have" as it only states they aren't allowed unescaped. Since there is a configuration for grpc-gateway that allows you to accept escaped path parameters, then this is still viable even without OpenAPI support. |
That’s interesting. We use ESP, which doesn’t provide that: https://cloud.google.com/endpoints/docs/grpc/specify-esp-v2-startup-options#transcoding |
:-) you could always switch. |
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. |
🚀 Feature
This is a follow-up to PR #2562
After attempting to implement this change in our codebase, unfortunately this doesn't help my team very much.
We use the
additional_mappings
attribute of the google.api.http extension to provide alternative path parameter formats for a required parameter of the entity which means depending on theadditional_mappings
the name of the parameter should be different.In the below example we have a
Foo
resource that has aparent
property of which abing/*
orbash/*
resource name is allowed for the parent.This generates two different OpenAPI path mappings, one for
POST /v1/{parent}/Foo
and one forPOST /v1/{parent_1}/Foo
. Unfortunately one of them has the path parameter regex restriction for "bash/" and the other "bing/" and there is nothing visible in the Swagger UI of a difference between them (you just have very confused users why they can use one and not the other).There are two acceptable outputs IMHO:
Either two endpoints
POST /v1/{bingParent}/Foo
andPOST /v1/{bashParent}/Foo
. This is unfortunately impossible with the current design for the path name replacements as you annotate the field on the Message which is the same between both.OR
One endpoint
POST /v1/{parent}/Foo
with theparent
parameter being some kind of regular expression combination that allows eitherbing/*
orbash/*
. This would require some really smart processing to determine that everything about the URLs is identical except the restriction on the path parameter and combine the two into a single OpenAPI endpoint. This seems like the "more correct" solution and would greatly simplify our output. This would be a change to my prior PR that adds "_1" suffixes on the path parameters to instead merge together two identical endpoints and make the path parameter regex support both standards.Thoughts?
The text was updated successfully, but these errors were encountered: