Skip to content
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

No HTTP transcoding for batch get nested request objects? #1380

Open
bell-db opened this issue Jul 8, 2024 · 4 comments
Open

No HTTP transcoding for batch get nested request objects? #1380

bell-db opened this issue Jul 8, 2024 · 4 comments

Comments

@bell-db
Copy link

bell-db commented Jul 8, 2024

AIP-231 allows nested request objects if

a field besides the resource name needs to be different between different resources being requested.

message BatchGetBooksRequest {
  // The requests specifying the books to retrieve.
  // A maximum of 1000 books can be retrieved in a batch.
  repeated GetBookRequest requests = 2
    [(google.api.field_behavior) = REQUIRED];
}

The same AIP also says

There must not be a body key in the google.api.http annotation.

Can we infer that requests are filled with query parameters? However, there seems no HTTP transcoding for repeated messages in query params:

Note that fields which are mapped to URL query parameters must have a primitive type or a repeated primitive type or a non-repeated message type.

@noahdietz
Copy link
Collaborator

Hi there, sorry for the late reply. You're right these things are at odds and when the guidance was changed in #281 to provide the "nested request message" as an alternative it was not updated to also say "use POST" though that is what was suggested and agreed upon in the relevant discussion: #203 (comment)

I think we need to change that...

@noahdietz
Copy link
Collaborator

In looking at all public GCP APIs, none of them have a nested request message BatchGet, so I think we should be good from our end to update the guidance without worrying about conflicting examples.

@bell-db
Copy link
Author

bell-db commented Jan 17, 2025

Got it. Thanks for looking into it. Is there a PR to update AIP-231 according to this guideline?

@noahdietz
Copy link
Collaborator

Got it. Thanks for looking into it. Is there a PR to update AIP-231 according to this guideline?

There isn't one yet, no, please feel free to open one and ping me on it!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants