feat: AIP-151 – Long-running operations by lukesneeringer · Pull Request #17 · aip-dev/aip.dev

lukesneeringer · 2020-11-21T00:46:11Z

This AIP presented some interesting issues around wording: At @mkistler's request, we were using the term "operation" where AIPs previously used "method".

I think we might need to use the term "endpoint" for that purpose so that we can use "operation" for this one. Initially, I tried to use "execution" for the concept needed here, but it did not work well.

@mkistler

This AIP presented some interesting issues around wording: At @mkistler's request, we were using the term operation where AIPs previously used method. I think we might need to use the term endpoint for that purpose so that we can use operation for this one.

lukesneeringer · 2020-11-21T00:46:45Z

Note to self: Fleshed out sample still needed.

tinnou · 2021-01-12T19:07:49Z

+}
+```
+
+- If the `done` field is `true`, then one and exactly one of the `response` and


(forgot to submit) Should we care about partial responses? Meaning the operation partially succeeded and returned some of the data with an error.
In that case, should we specify that StatusMonitor.error should only encompass errors that aborted the entire operation?

tinnou · 2021-01-12T19:46:47Z

+error response (AIP-193), similar to any other method.
+
+Errors that occur over the course of an request **may** be placed in the
+metadata message. The errors themselves **must** still be represented with a


In what cases should the error be placed in StatusMonitor.metadata but not on StatusMonitor.error?

Partial failures, for sure.

tinnou · 2021-01-12T19:51:48Z

+    _never_ be needed. If response data might be added in the future, define an
+    empty message for the RPC response and use that.
+  - The metadata type is used to provide information such as progress, partial
+    failures, and similar information on each `GetOperation` call. The metadata


oh I see I should have read until the end, so the guidance if I understand correctly:

A. Errors preventing the operation to start -> (AIP-193)
B. Error making the entire request unsuccessful, (e.g a 500 on a downstream service) -> StatusMonitor.error populated, with optionally error metadata in StatusMonitor.metadata. StatusMonitor.response must not be populated.
C. Any partial failures -> StatusMonitor.response populated, and error metadata in StatusMonitor.metadata. StatusMonitor.error must not be populated.
D. Successful request no errors -> StatusMonitor.response populated, and optionally metadata in StatusMonitor.metadata. StatusMonitor.error must not be populated.

google-cla · 2021-01-18T22:04:49Z

All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter.

We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only @googlebot I consent. in this pull request.

Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the cla label to yes (if enabled on your project).

ℹ️ Googlers: Go here for more info.

google-cla · 2021-01-19T12:32:49Z

All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter.

We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only @googlebot I consent. in this pull request.

Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the cla label to yes (if enabled on your project).

ℹ️ Googlers: Go here for more info.

google-cla · 2021-01-19T18:06:20Z

All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter.

We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only @googlebot I consent. in this pull request.

Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the cla label to yes (if enabled on your project).

ℹ️ Googlers: Go here for more info.

mkistler · 2021-01-25T12:46:57Z

A couple of points to discuss on this one:

The description of the result field does not jive with the TypeScript rendering, at least for me. I would have rendered result as an actual property whose value is an object containing either a response property or a error property.
Rather than define a field called “metadata”, it will be more natural in a OpenAPI definition to allow StatusMonitor to have “additionalProperties”. You can represent this in Typescript with

interface StatusMonitor {
  < other fields >

  // Additional properties associated with the request.
  // Populated throughout the life of the request, including after
  // it completes.
  [x: string]: any 
}

google-cla · 2021-01-25T23:25:35Z

All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter.

We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only @googlebot I consent. in this pull request.

Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the cla label to yes (if enabled on your project).

ℹ️ Googlers: Go here for more info.

lukesneeringer · 2021-01-25T23:26:39Z

Rather than define a field called “metadata”, it will be more natural in a OpenAPI definition to allow StatusMonitor to have “additionalProperties”.

This might be my own pre-conceived biases, but this feels worse to me than a clear structure where all the "additional properties" go. It also ensures that the "base" structure can have new properties added to the main struct in a backwards-compatible way.

google-cla · 2021-02-02T18:35:46Z

All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter.

We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only @googlebot I consent. in this pull request.

Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the cla label to yes (if enabled on your project).

ℹ️ Googlers: Go here for more info.

google-cla · 2021-02-09T14:35:35Z

All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter.

We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only @googlebot I consent. in this pull request.

Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the cla label to yes (if enabled on your project).

ℹ️ Googlers: Go here for more info.

mkistler

Looks good! 👍

lukesneeringer requested a review from a team as a code owner November 21, 2020 00:46

Luke Sneeringer added 5 commits November 23, 2020 13:19

Shift to a complete example.

587091b

Fix bug in the sample.

4ba40b9

Rewording.

f26ddf9

Status monitor

1a8fb70

Status monitor redux

8c0438a

tinnou reviewed Jan 12, 2021

View reviewed changes

Comment thread aip/general/0151/aip.md.j2 Outdated

tinnou reviewed Jan 12, 2021

View reviewed changes

Add oas interface defintion

1d0246e

More oas changes

9801270

Merge branch 'master' into aip-151

0ce02b3

Incorporate @tinnou feedback.

c79ac73

Merge branch 'master' into aip-151

d1eb592

lukesneeringer added the cla: yes label Feb 2, 2021

Update oas example for consistency w/ protobuf

ed5b4d5

google-cla Bot added cla: no and removed cla: yes labels Feb 9, 2021

lukesneeringer removed the cla: no label Feb 9, 2021

lukesneeringer added the cla: yes label Feb 9, 2021

mkistler approved these changes Feb 9, 2021

View reviewed changes

lukesneeringer merged commit edbc2cc into master Feb 10, 2021

lukesneeringer deleted the aip-151 branch February 10, 2021 22:06

Conversation

lukesneeringer commented Nov 21, 2020

Uh oh!

lukesneeringer commented Nov 21, 2020

Uh oh!

tinnou Jan 12, 2021

Choose a reason for hiding this comment

Uh oh!

Uh oh!

tinnou Jan 12, 2021

Choose a reason for hiding this comment

Uh oh!

lukesneeringer Jan 25, 2021

Choose a reason for hiding this comment

Uh oh!

tinnou Jan 12, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

google-cla Bot commented Jan 18, 2021

Uh oh!

google-cla Bot commented Jan 19, 2021

Uh oh!

google-cla Bot commented Jan 19, 2021

Uh oh!

mkistler commented Jan 25, 2021

Uh oh!

google-cla Bot commented Jan 25, 2021

Uh oh!

lukesneeringer commented Jan 25, 2021

Uh oh!

google-cla Bot commented Feb 2, 2021

Uh oh!

google-cla Bot commented Feb 9, 2021

Uh oh!

mkistler left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

tinnou Jan 12, 2021 •

edited

Loading