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

Is OUTPUT_ONLY a correct annotation for name field ? #318

Closed
glimchb opened this issue Aug 2, 2023 · 7 comments · Fixed by #387
Closed

Is OUTPUT_ONLY a correct annotation for name field ? #318

glimchb opened this issue Aug 2, 2023 · 7 comments · Fixed by #387
Assignees

Comments

@glimchb
Copy link
Member

glimchb commented Aug 2, 2023

          Is `OUTPUT_ONLY` a correct annotation here?

This is a hard one... According to the annotation description:

including the field in a message in a request does nothing (the server must clear out any value in this field and must not throw an error as a result of the presence of a value in this field on input). Similarly, services must ignore the presence of output only fields in update field masks.

Based on that, a user cannot specify a name, since the server must ignore it. But this resource is used in Update call and name is used to find that resource internally...

This might be a resolution: 2 weeks ago a PR from an aip team member was created where it is said that name field must not be annotated.

Originally posted by @artek-koltun in #317 (comment)

@glimchb
Copy link
Member Author

glimchb commented Aug 2, 2023

check also

 // Example: `projects/foo/locations/bar/questions/123`
  string name = 1 [
    (google.api.field_behavior) = OUTPUT_ONLY,
    (google.api.field_behavior) = IMMUTABLE
  ];

from https://github.com/googleapis/googleapis/blob/449686b58eaee7df382c2020f5069769deebde17/google/cloud/dataqna/v1alpha/question.proto#L43-L48

@jainvipin
Copy link
Contributor

jainvipin commented Aug 3, 2023

should we go with just this for now?

    (google.api.field_behavior) = IMMUTABLE

@artek-koltun
Copy link
Contributor

aip-dev/google.aip.dev#1201

@glimchb
Copy link
Member Author

glimchb commented Aug 28, 2023

sounds promising... waiting for this to be merged...

  string name = 1 [(google.api.field_behavior) = IDENTIFIER];

@noahdietz
Copy link

aip-dev/google.aip.dev#1201 is merged 😉 linter rules to follow if y'all use it...

@glimchb
Copy link
Member Author

glimchb commented Aug 31, 2023

new linter PR to enforce IDENTIFIER is merged googleapis/api-linter#1241
waiting on the new release https://github.com/googleapis/api-linter/releases
comes from googleapis/api-linter#1243

@glimchb glimchb linked a pull request Aug 31, 2023 that will close this issue
@glimchb glimchb self-assigned this Sep 7, 2023
@noahdietz
Copy link

noahdietz commented Sep 11, 2023

The rules are now available in: https://github.com/googleapis/api-linter/releases/tag/v1.57.0

Edit: typo

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