fix: gapic showcase proto compliance #1591
Conversation
|
|
||
| message PagedExpandLegacyMappedResponse { | ||
| // The words that were expanded, indexed by their initial character. | ||
| // (-- aip.dev/not-precedent: This is a legacy, non-standard pattern that violates |
There was a problem hiding this comment.
I don't think we want to remove this. GCE uses this type of "mapped-paged" response in several RPCs, which is why this RPC was put in Showcase. How does PHP handle building the GCE GAPIC?
There was a problem hiding this comment.
That's interesting, I don't think we handle map for pagination in GCE. That may be a bug on our end.
Would you be able to point to an example in the protos (or in a doc) where this exists and how we should be handling it?
There was a problem hiding this comment.
The error message seems to imply this is legacy and non-standard, which is probably why we don't support it
| post: "/v1beta1/users" | ||
| body: "*" | ||
| }; | ||
| option (google.api.method_signature) = "user.display_name,user.email"; |
There was a problem hiding this comment.
These are legitimate method signatures. See https://google.aip.dev/client-libraries/4232#method-signatures_1 :
If a field's name contains a period (.) character, this indicates a nested field.An RPC can provide this annotation more than once to specify multiple signatures. Order matters here
There was a problem hiding this comment.
I don't think we handle this in the PHP generator either, and it seems like we should (if it's an AIP). However, it's interesting that this hasn't caused issues to us, so I'd guess in practice it isn't being used.
| pattern: "users/{user}/blurbs/{blurb}" | ||
| pattern: "rooms/{room}/blurbs/{blurb}" | ||
| pattern: "rooms/{room}/blurbs/legacy/{legacy_room}.{blurb}" | ||
| pattern: "rooms/{room}/legacy_room/{legacy_room}/blurbs/{blurb}" |
There was a problem hiding this comment.
Why are these modified/added resource names needed? (I'll need to understand resource names more deeply in general.)
There was a problem hiding this comment.
I just know they fail when we build them in the format you've provided here. I also don't know enough about them to say if they're valid, but I am assuming they aren't (since PHP's generator works on all the other APIs but this one)
cc @noahdietz who may know more about these
2 participants
These are fixes that were required for the PHP client libraries to build. Notably they:
method_signatureannotationsresource.patternrepeatedinstead ofmapfor pagination