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

Disallow "piggybacking" the etag field #1236

Open
simonbos opened this issue Oct 3, 2023 · 0 comments
Open

Disallow "piggybacking" the etag field #1236

simonbos opened this issue Oct 3, 2023 · 0 comments

Comments

@simonbos
Copy link

simonbos commented Oct 3, 2023

Context
I feel there is currently some confusing and conflicting guidance for the etag field:

  • AIP-203 mentions each field MUST include a field behavior, whereas AIP-154 mentions the etag field SHOULD NOT have a field behavior.
  • Analogous to the previous point, AIP-134: etag behavior annotation guidance unclear and conflicting #952 also noted that the field behavior for the etag field is conflicting in AIP-134 and AIP-154.
  • AIP-135: the method_signature of the Delete RPC MAY include the etag field, whereas I didn't find it in other AIPs.
  • AIP-134 includes some guidance on the update mask specific for only the etag field: "The update_mask field in the request does not affect the behavior of the etag field, as it is not a field being updated."
  • AIP-154 mentions "In some situations, the etag needs to belong on a request message rather than the resource itself", but only gives one specific example (Update).

Discussion
I believe this conflicting guidance originates by allowing "piggybacking" of the etag field (which seems to only be used for the Update RPC). By disallowing this "piggybacking", I believe the guidance would become clearer:

  1. The Update RPC (and any other example using "piggybacking" at the moment - if there are any other examples) would include the etag on the request message:
message UpdateBookRequest {
  // The book to update.
  //
  // The book's `name` field is used to identify the book to update.
  // Format: publishers/{publisher}/books/{book}
  Book book = 1 [(google.api.field_behavior) = REQUIRED];

  // The list of fields to update.
  google.protobuf.FieldMask update_mask = 2;

  // The current etag of the book.
  // If an etag is provided and does not match the current etag of the book,
  // the update will be blocked and an ABORTED error will be returned.
  string etag = 3 [(google.api.field_behavior) = OPTIONAL];
}
  1. The field behavior of etag on a resource would be OUTPUT_ONLY.
  2. The method_signature of RPCs would include the "etag" if it is required on the request.
  3. All special cases and mentions I highlighted would become obsolete.
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

1 participant