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-154: etag field_behavior #1395

Closed
loeffel-io opened this issue Jun 4, 2024 · 3 comments · Fixed by #1412
Closed

AIP-154: etag field_behavior #1395

loeffel-io opened this issue Jun 4, 2024 · 3 comments · Fixed by #1412

Comments

@loeffel-io
Copy link
Contributor

AIP-154 mentions

Etags on request methods

On a request message, the etag field should be given a behavior annotation - either REQUIRED or OPTIONAL. See AIP-203 for more information.

But there is a linter error when using REQUIRED:

- file_path: mindful/earth/user/v1/user.proto
  problems:
    - message: Delete RPCs must only require fields explicitly described in AIPs, not "etag".
      location:
        start_position:
            line_number: 282
            column_number: 3
        end_position:
            line_number: 282
            column_number: 59
        path: mindful/earth/user/v1/user.proto
      rule_id: core::0135::request-required-fields
      rule_doc_uri: https://linter.aip.dev/135/request-required-fields

message:

message DeleteUserRequest {
  // The resource name
  // Format: users/{user}
  string name = 1 [
    (google.api.resource_reference) = {type: "user.mindful.com/User"},
    (google.api.field_behavior) = REQUIRED,
    (validate.rules).string = {pattern: "^users\\/[a-f0-9]{8}-[a-f0-9]{4}-[a-f0-9]{4}-[a-f0-9]{4}-[a-f0-9]{12}"}
  ];
  // The etag of the user. Must match the server's etag.
  string etag = 2 [(google.api.field_behavior) = REQUIRED];
}
@noahdietz
Copy link
Collaborator

Yep, you are definitely right. Sorry for not seeing this bug earlier, it's been a bit crazy.

Fixing in #1414 and looking at the other RPCs

@noahdietz
Copy link
Collaborator

We had an internal bug open to look at this for Standard Get as well. I won't proceed with the change yet as there is still some debate there. This will be marked fixed when I release the ApiLinter, which will happen on Monday (no Friday releases) reviews permitting.

@noahdietz noahdietz self-assigned this Aug 8, 2024
gcf-merge-on-green bot pushed a commit that referenced this issue Aug 9, 2024
Allow Standard Field `etag` to be `REQUIRED` in Standard Delete.

Updates #1395
@loeffel-io
Copy link
Contributor Author

Thanks @noahdietz 🙏

gcf-merge-on-green bot pushed a commit that referenced this issue Aug 14, 2024
🤖 I have created a release *beep* *boop*
---


## [1.67.2](https://togithub.com/googleapis/api-linter/compare/v1.67.1...v1.67.2) (2024-08-14)


### Bug Fixes

* **AIP-123:** multiword singleton reduction ([#1417](https://togithub.com/googleapis/api-linter/issues/1417)) ([7868552](https://togithub.com/googleapis/api-linter/commit/7868552ff7b27c2fa0f2ff9be3a538763f0450c5))
* **AIP-135:** allow required etag in Delete ([#1414](https://togithub.com/googleapis/api-linter/issues/1414)) ([aa9587b](https://togithub.com/googleapis/api-linter/commit/aa9587bc7184a78109f138c809baa00018ea75e9)), refs [#1395](https://togithub.com/googleapis/api-linter/issues/1395)
* **AIP-235:** allow hosting allow_missing ([#1416](https://togithub.com/googleapis/api-linter/issues/1416)) ([6bfbcdf](https://togithub.com/googleapis/api-linter/commit/6bfbcdfa8858ccdba98760d76e2d2a757855cc7b)), refs [#1404](https://togithub.com/googleapis/api-linter/issues/1404)
* exit rule if response type cannot be resolved ([#1415](https://togithub.com/googleapis/api-linter/issues/1415)) ([6874dab](https://togithub.com/googleapis/api-linter/commit/6874dabb4f0d3503f267bb0ab970d62785d12727)), refs [#1399](https://togithub.com/googleapis/api-linter/issues/1399)


### Documentation

* **AIP-143:** fix rule name used for implementation link ([#1411](https://togithub.com/googleapis/api-linter/issues/1411)) ([f9cf2eb](https://togithub.com/googleapis/api-linter/commit/f9cf2ebc9589abfce88317b1e3318a9e1547b41a))

---
This PR was generated with [Release Please](https://togithub.com/googleapis/release-please). See [documentation](https://togithub.com/googleapis/release-please#release-please).
@noahdietz noahdietz linked a pull request Aug 14, 2024 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants