fix(internal/gengapic): add workaround for operation collision #1397
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
We have a service that is not yet released that uses the same RPC name across two services in the same proto package. Our Operation wrapper identifiers are not scoped to a service so this causes issues when generated a client that does this. For now this is for one client, which may change before GA, so a hacky solution is fine. But we should keep this in the back of our minds should we ever v2 our libraries this is something we would want to avoid.
noahdietz
approved these changes
Sep 21, 2023
quartzmo
reviewed
Sep 21, 2023
quartzmo
reviewed
Sep 21, 2023
| if eHTTP, ok := proto.GetExtension(m.GetOptions(), annotations.E_Http).(*annotations.HttpRule); ok && eHTTP != nil && eHTTP.Pattern != nil { | ||
| switch t := eHTTP.Pattern.(type) { | ||
| case *annotations.HttpRule_Post: | ||
| if t.Post == "/v1beta1/{parent=projects/*/locations/*/featureGroups/*}/features" { |
There was a problem hiding this comment.
It seems at least a tiny bit possible that someday another RPC will match this pattern...
There was a problem hiding this comment.
It may, but I believe it is unlikely. Ideally this code gets deleted in the next couple of months. If we end up having to do this for the v1 service we can make the logic tighter at that time. I was trying to avoid passing a lot more info throughout the stack if possible so this is easy to unwind.
quartzmo
approved these changes
Sep 21, 2023
codyoss
added a commit
that referenced
this pull request
Sep 22, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
We have a service that is not yet released that uses the same RPC name across two services in the same proto package. Our Operation wrapper identifiers are not scoped to a service so this causes issues when generated a client that does this. For now this is for one client, which may change before GA, so a hacky solution is fine. But we should keep this in the back of our minds should we ever v2 our libraries this is something we would want to avoid.