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

AIP-136 is missing guidance about when to use {name= #955

Open
rofrankel opened this issue Oct 14, 2022 · 6 comments
Open

AIP-136 is missing guidance about when to use {name= #955

rofrankel opened this issue Oct 14, 2022 · 6 comments

Comments

@rofrankel
Copy link
Contributor

In this linter issue, @noahdietz asked me to file an issue to update AIP-136 with guidance about when to use {name= in a google.api.http annotation.

Contrary to the linter's core::136::http-name-variable rule, the actual AIP has nothing to say about not using name= in the URI. It does say "The name of the method should be a verb followed by a noun.", but it doesn't say not to use name= in the URI when there's a good reason not to follow that guidance.

@noahdietz
Copy link
Collaborator

AFAICT the pattern is to use name/parent for stateful methods e.g. MutateTable, and {resource} e.g. book or project for stateless methods - when the RPC runs in the context of, but not directly dependent on the state of, a given resource.

@noahdietz
Copy link
Collaborator

Also, for clarity, this issue manifests in the linting of the google.api.http annotation, but it is directly related to the field name used in the request message for a field that represents a resource name and that is also used in the HTTP pattern.

@noahdietz
Copy link
Collaborator

Just an update, we discussed this as a team and are still trying to come to a consensus. Hope to have something next week for you.

@jgeewax
Copy link
Contributor

jgeewax commented Oct 31, 2022

Hi folks,

IIRC while standard methods use name as the request field to store the unique identifier for the resource under operation (or parent in the case of Create), custom methods are a bit more flexible (and therefore ambiguous). You can be operating on a specific resource (e.g., POST /rockets/1234:launch) or a collection (e.g., POST /publishers/1234/books:sort). You can also be using custom methods in stateless scenarios, where the parent is more involved for purposes of quota or billing (e.g., POST /projects/1234:translateText).

Because of this, and the ambiguity of whether to use name or parent, we opted to use name in cases when you're dealing with a singular resource that lines up very closely to Get, and then use the singular lowercase name of the resource for all other categories of custom methods.

This means we'd have:

  • POST /{name=rockets/*}:launch (instead of rocket=rockets/*)
  • POST /{publisher=publishers/*}/books:sort (instead of parent=publishers/* because this is a "collection-based" custom method)
  • POST /{project=projects/*}:translateText (instead of name=projects/* or parent=projects/* because this is a stateless custom method)

It's possible the linter rule made an assumption that custom methods were like the standard methods (because of the first example in AIP-136 being Archive) and it's only now being noticed. We should probably remove this check while we work on replacement checks, but it could be quite challenging to do these checks properly...

@noahdietz
Copy link
Collaborator

Thanks JJ, that's really helpful context. I will disable the checks in the api-linter (and follow up on the linter issue itself).

I would like to leave this open so that we can distill what you've laid out here into some guidance for AIP-136.

@rofrankel
Copy link
Contributor Author

Oops, just noticed that I forgot to respond to this, even though I did take action based on it. Thanks JJ and Noah for the answers here.

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

3 participants