opiproject / opi-api

Open Programmable Infrastructure API and Behavioral Model
Apache License 2.0
31 stars 40 forks source link

Is `OUTPUT_ONLY` a correct annotation for `name` field ? #318

Closed glimchb closed 11 months ago

glimchb commented 1 year ago
          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 https://github.com/opiproject/opi-api/pull/317#discussion_r1281526799_

glimchb commented 1 year ago

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 commented 1 year ago

should we go with just this for now?

    (google.api.field_behavior) = IMMUTABLE
artek-koltun commented 1 year ago

https://github.com/aip-dev/google.aip.dev/pull/1201

glimchb commented 1 year ago

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

  string name = 1 [(google.api.field_behavior) = IDENTIFIER];
noahdietz commented 1 year ago

https://github.com/aip-dev/google.aip.dev/pull/1201 is merged 😉 linter rules to follow if y'all use it...

glimchb commented 1 year ago

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

noahdietz commented 12 months ago

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

Edit: typo