feat(AIP-203): clarify immutable optional, required fields #1120

toumorokoshi · 2023-05-24T17:14:18Z

Previous guidance had OPTIONAL as a singular value that had to be mutually exclusive with every other annotation. This eliminates the ability to express fine-grained details like OPTIONAL + INPUT_ONLY.

In a similar vain, IMMUTABLE does not imply required, nor input.

Clarifying which annotations are required based on these update.

fixes #1119

Previous guidance had OPTIONAL as a singular value that had to be mutually exclusive with every other annotation. This eliminates the ability to express fine-grained details like OPTIONAL + INPUT_ONLY. In a similar vain, IMMUTABLE does not imply required, nor input. Clarifying which annotations are required based on these update.

aip/general/0203.md

removed missed wording on optional.

lukesneeringer · 2023-05-24T21:27:15Z

aip/general/0203.md

@@ -171,6 +163,15 @@ A resource with an unordered list **may** return the list in a stable order, or

 ## Rationale

+### Required set of annotations
+
+A field used in a request message must be either an input or an output.


What does this mean?

This section explains why some combination of annotations are required, and was trying to explain that a field is either and input or and output, and therefore a full description of the behavior should include at least one of required, optional, or output_only.

lukesneeringer · 2023-05-24T21:27:28Z

aip/general/0203.md

@@ -171,6 +163,15 @@ A resource with an unordered list **may** return the list in a stable order, or

 ## Rationale

+### Required set of annotations


I don't think this section should exist.

Can you clarify which section, and clarify why you think it shouldn't exist? If you're referring to the rationale section, there was several requests internally and unanimous support among the current API design team. Public discussion is at: #1058.

lukesneeringer · 2023-05-24T21:28:12Z

aip/general/0203.md

+
+In the case of an output, the `OUTPUT_ONLY` annotation is sufficient.
+
+In the case of an input, a field is either required or optional, and therefore


We specifically didn't want to mandate OPTIONAL. (There was debate on whether to even provide it.)

Yes, and after a decent amount of discussion it was agreed that field_behavior should be required: #1100. See the rationale section for more details, but this choice has lead to a lot of incorrectly documented fields since developers did not think through the field behavior choice. field behavior is being used as forcing function.

lukesneeringer · 2023-05-24T21:29:31Z

aip/general/0203.md

@@ -24,10 +24,12 @@ field behavior, such as a field being required or immutable.
 RecognitionAudio audio = 2 [(google.api.field_behavior) = REQUIRED];
 ```

- APIs **must** apply the `google.api.field_behavior` annotation on every field.
+- APIs **must** apply the `google.api.field_behavior` annotation on every field


When did this become a requirement? It shouldn't be one. Leaving off a field_behavior annotation is equivalent to specifying OPTIONAL.

See #1100, there's a separate thread you opened we could discuss this too.

lukesneeringer · 2023-05-24T21:32:21Z

aip/general/0203.md

@@ -142,20 +148,6 @@ Potential use cases for immutable fields (this is not an exhaustive list) are:
 **Note:** Fields which are "conditionally immutable" **must not** be given the
 immutable annotation.

-### Optional
-
-The use of `OPTIONAL` indicates that a field is not required, nor covered by


The reason that "not covered by any of the above descriptions" was included is because when the annotation was being debated, API producers wanted to avoid the outcome of having to specify field_behavior on every field. We almost didn't provide OPTIONAL at all, but Docgen wanted it for cases where documentation needed to call out that the field was optional (e.g. if it was counter-intuitive).

Great, that matches my understanding as well. There's been a lot of discussion internally around this and the conclusion (with ATL review) was that we should require these, as the cost to API producers is worth the benefit to clients.

toumorokoshi · 2023-05-25T21:59:26Z

@lukesneeringer thanks! I really appreciate the time to review.

I answered your questions, so hopefully that provides context. Let me know if you have blockers. Regarding the requirement of field_behavior overall, that's a decision we made internally (that this CL is intended to publicize).

toumorokoshi · 2023-05-31T17:55:05Z

@lukesneeringer gentle ping. Since we have agreement internally to move forward with this, and all current Googlers approve, I'm inclined to merge this.

If you think there is a strong technical reason, I'm definitely open to talking those out.

lukesneeringer · 2023-05-31T21:26:48Z

@toumorokoshi Sure, that's fine. I'm not particularly concerned about whether field_behavior is mandatory since, well, I don't work at Google anymore and thus am not affected by the decision. :-)

toumorokoshi · 2023-06-01T04:29:57Z

@toumorokoshi Sure, that's fine. I'm not particularly concerned about whether field_behavior is mandatory since, well, I don't work at Google anymore and thus am not affected by the decision. :-)

Of course, and I think despite your xoogler status, your opinion is still very valuable.

Thank you for taking our time to review! I do really appreciate it.

toumorokoshi requested a review from a team as a code owner May 24, 2023 17:14

noahdietz approved these changes May 24, 2023

View reviewed changes

aip/general/0203.md Show resolved Hide resolved

bgrant0607 reviewed May 24, 2023

View reviewed changes

aip/general/0203.md Outdated Show resolved Hide resolved

address feedback

dec6035

removed missed wording on optional.

toumorokoshi mentioned this pull request May 24, 2023

feat(AIP-203): require field_behavior googleapis/api-linter#1149

Merged

bgrant0607 approved these changes May 24, 2023

View reviewed changes

lukesneeringer suggested changes May 24, 2023

View reviewed changes

toumorokoshi merged commit f896944 into aip-dev:master Jun 1, 2023

toumorokoshi mentioned this pull request Jun 1, 2023

AIP-203: fix up linting rules that allow more than optional googleapis/api-linter#1167

Open

toumorokoshi mentioned this pull request Jul 21, 2023

IMMUTABLE + OUTPUT_ONLY field behavior #317

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(AIP-203): clarify immutable optional, required fields #1120

feat(AIP-203): clarify immutable optional, required fields #1120

toumorokoshi commented May 24, 2023

lukesneeringer May 24, 2023

toumorokoshi May 24, 2023

lukesneeringer May 24, 2023

toumorokoshi May 24, 2023

lukesneeringer May 24, 2023

toumorokoshi May 24, 2023

lukesneeringer May 24, 2023

toumorokoshi May 24, 2023 •

edited

Loading

lukesneeringer May 24, 2023

toumorokoshi May 24, 2023 •

edited

Loading

toumorokoshi commented May 25, 2023

toumorokoshi commented May 31, 2023

lukesneeringer commented May 31, 2023

toumorokoshi commented Jun 1, 2023

		@@ -171,6 +163,15 @@ A resource with an unordered list may return the list in a stable order, or

		## Rationale

		### Required set of annotations


		In the case of an output, the `OUTPUT_ONLY` annotation is sufficient.

		In the case of an input, a field is either required or optional, and therefore

feat(AIP-203): clarify immutable optional, required fields #1120

feat(AIP-203): clarify immutable optional, required fields #1120

Conversation

toumorokoshi commented May 24, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

toumorokoshi May 24, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

toumorokoshi May 24, 2023 • edited Loading

Choose a reason for hiding this comment

toumorokoshi commented May 25, 2023

toumorokoshi commented May 31, 2023

lukesneeringer commented May 31, 2023

toumorokoshi commented Jun 1, 2023

toumorokoshi May 24, 2023 •

edited

Loading

toumorokoshi May 24, 2023 •

edited

Loading