From a3fa2470e5af2aa6c72be7cb7442bc3a973f101e Mon Sep 17 00:00:00 2001 From: Richard Frankel Date: Fri, 8 Mar 2024 15:56:42 -0600 Subject: [PATCH 1/4] Clean up and modernize AIP-151: Long-running operations. The changes are pretty broad, and include: 1) Refer to operations rather than status monitors. This is based on discussion in Slack with Luke Sneeringer, who originally adopted this AEP. 2) Add references to the common components repo (aep-dev/aep) and perform corresponding cleanup in the examples. This PR corresponds to https://github.com/aep-dev/aep/pull/8. --- aep/general/0151/aep.md.j2 | 109 ++++++++++++++-------------------- aep/general/0151/lro.oas.yaml | 21 +++++-- aep/general/0151/lro.proto | 6 +- 3 files changed, 63 insertions(+), 73 deletions(-) diff --git a/aep/general/0151/aep.md.j2 b/aep/general/0151/aep.md.j2 index 29c1f943..2cea9424 100644 --- a/aep/general/0151/aep.md.j2 +++ b/aep/general/0151/aep.md.j2 @@ -1,4 +1,4 @@ -# Long-running requests +# Long-running operations Occasionally, a service may need to expose an operation that takes a significant amount of time to complete. In these situations, it is often a poor @@ -13,77 +13,50 @@ can be used to track progress and retrieve the result. ## Guidance Operations that might take a significant amount of time to complete **should** -return a `202 Accepted` response along with an identifier that can be used to -track the status of the request and ultimately retrieve the result. +return a `202 Accepted` response along with an `Operation` resource that can be +used to track the status of the request and ultimately retrieve the result. Any single operation defined in an API surface **must** either _always_ return -`202 Accepted` along with a request identifier, or _never_ do so. A service -**must not** return a `200 OK` response with the result if it is "fast enough", -and `202 Accepted` if it is not fast enough, because such behavior adds -significant burdens for clients. +`202 Accepted` along with an `Operation`, or _never_ do so. A service **must +not** return a `200 OK` response with the result if it is "fast enough", and +`202 Accepted` if it is not fast enough, because such behavior adds significant +burdens for clients. **Note:** User expectations can vary on what is considered "a significant amount of time" depending on what work is being done. A good rule of thumb is 10 seconds. -### Status monitor representation +### Operation representation -The response to a long-running request **should** be a "status monitor" having -the following common format: +The response to a long-running request **must** be an [`Operation`][]. -```typescript -interface StatusMonitor { - // The identifier for this status monitor. - id: string; +{% tab proto %} - // Whether the request is done. - done: boolean; +Protocol buffer APIs **must** use the common component [`aep.api.Operation`][]. - // The result of the request. - // Only populated if the request is done and was successful. - response: any; +{% tab oas %} - // The error that arose from the request. - // Only populated if the request is done and was unsuccessful. - error: Error; +OpenAPI services **must** use this [`JSON Schema Operation`][] schema. - // Metadata associated with the request. - // Populated throughout the life of the request, including after - // it completes. - metadata: any; -} -``` +{% endtabs %} -- If the `done` field is `true`, then one and exactly one of the `response` and - `error` fields **must** be populated. - - If the `done` field is `false`, then the `response` and `error` fields - **must not** be populated. -- The `response` and `metadata` fields **may** be any type that the service - determines to be appropriate, but **must** always be the same type for any - particular operation. - - The `response` and `metadata` types **should** be defined in the same API - surface as the operation itself. - - The `response` and `metadata` types that need no data **should** use a - custom-defined empty struct rather than a common void or empty type, to - permit future extensibility. - -### Querying a status monitor +### Querying an operation The service **must** provide an endpoint to query the status of the operation, -which **must** accept the operation identifier and **should not** include other +which **must** accept the operation path and **should not** include other parameters: ```http -GET /v1/statusMonitors/{status_monitor} HTTP/2 -Host: library.googleapis.com +GET /v1/operations/{operation} HTTP/2 +Host: library.example.com Accept: application/json ``` -The endpoint **must** return a `StatusMonitor` as described above. +The endpoint **must** return a `Operation` as described above. ### Standard methods -APIs **may** return an `StatusMonitor` from the [`Create`][aip-133], +APIs **may** return an `Operation` from the [`Create`][aip-133], [`Update`][aip-134], or [`Delete`][aip-135] standard methods if appropriate. In this case, the `response` field **must** be the standard and expected response type for that standard method. @@ -107,10 +80,10 @@ but is not obligated to do so: ### Expiration -APIs **may** allow their status monitor resources to expire after sufficient -time has elapsed after the request completed. +APIs **may** allow their operation resources to expire after sufficient time +has elapsed after the request completed. -**Note:** A good rule of thumb for status monitor expiry is 30 days. +**Note:** A good rule of thumb for operation expiry is 30 days. ### Errors @@ -125,18 +98,17 @@ canonical error object. {% tab proto %} -When using protocol buffers, the well-known type `google.longrunning.Operation` -is used. - -**Note:** For historical reasons, Google uses the term `Operation` to represent -what this document describes as a `StatusMonitor`. +When using protocol buffers, the common component [`aep.api.Operation`][] is +used. {% sample 'lro.proto', 'rpc WriteBook' %} -- The response type **must** be `google.longrunning.Operation`. The `Operation` - proto definition **must not** be copied into individual APIs. +- The response type **must** be `aep.api.Operation`. The `Operation` proto + definition **should not** be copied into individual APIs; prefer to use a + single copy (in monorepo code bases), or remote dependencies via a tool like + [Buf][buf.build]. - The response **must not** be a streaming response. -- The method **must** include a `google.longrunning.operation_info` annotation, +- The method **must** include a [`aep.api.operation_info`][lro] annotation, which **must** define both response and metadata types. - The response and metadata types **must** be defined in the file where the RPC appears, or a file imported by that file. @@ -152,8 +124,10 @@ what this document describes as a `StatusMonitor`. metadata will _never_ be needed. If metadata might be added in the future, define an empty message for the RPC metadata and use that. - APIs with messages that return `Operation` **must** implement the - [`Operations`][lro] service. Individual APIs **must not** define their own - interfaces for long-running operations to avoid inconsistency. + `GetOperation` method of the [`Operations`][lro] service, and **may** + implement the other methods defined in that service. Individual APIs **must + not** define their own interfaces for long-running operations to avoid + inconsistency. {% tab oas %} @@ -162,8 +136,8 @@ what this document describes as a `StatusMonitor`. - `202` **must** be the only success status code defined. - The `202` response **must** define an `application/json` response body and no other response content types. -- The response body schema **must** be an object with `name`, `done`, and - `result` properties as described above for a StatusMonitor +- The response body schema **must** be an object with `path`, `done`, `error`, + and `response` properties as described above for an `Operation`. - The response body schema **may** contain an object property named `metadata` to hold service-specific metadata associated with the operation, for example progress information and common metadata such as create time. The service @@ -176,15 +150,18 @@ what this document describes as a `StatusMonitor`. as an empty object schema. For a standard `Get/Create/Update` operation, `response` should be a representation of the resource. - If a service has any long running operations, the service **must** define an - `StatusMonitor` resource with a `list` operation to retrieve a potentially - filtered list of status monitors and a `get` operation to retrieve a specific - status monitor by its `name`. + `Operation` resource with a `list` operation to retrieve a potentially + filtered list of operations and a `get` operation to retrieve a specific + operation by its `path`. {% endtabs %} [google.rpc.Status]: https://github.com/googleapis/api-common-protos/blob/master/google/rpc/S.proto -[lro]: https://github.com/googleapis/api-common-protos/blob/master/google/longrunning/operations.proto +[lro]: https://github.com/aep-dev/aep/blob/main/proto/aep-api/aep/api/operations.proto [node.js promise]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Guide/Using_promises [future]: https://docs.python.org/3/library/concurrent.futures.html#concurrent.futures.Future +[`Operation`]: https://github.com/aep-dev/aep/blob/main/schemas/api/operation.md +[`aep.api.Operation`]: https://buf.build/aep/api/docs/main:aep.api#aep.api.Operation +[`JSON Schema Operation`]: https://github.com/aep-dev/aep/blob/main/json_schema/api/operation.yaml diff --git a/aep/general/0151/lro.oas.yaml b/aep/general/0151/lro.oas.yaml index ed53e3b6..256013a6 100644 --- a/aep/general/0151/lro.oas.yaml +++ b/aep/general/0151/lro.oas.yaml @@ -17,13 +17,13 @@ paths: components: schemas: - StatusMonitor: + Operation: description: The status of a long running operation. properties: - name: + path: type: string description: - The server-assigned name, which is only unique within the same + The server-assigned path, which is only unique within the same service that originally returns it. done: type: boolean @@ -33,8 +33,21 @@ components: is available. error: $ref: '#/components/schemas/Error' + metadata: + type: object + description: >- + Service-specific metadata associated with the operation. It typically + contains progress information and common metadata such as create time. + Some services might not provide such metadata. Any method that returns a + long-running operation should document the metadata type, if any. + response: + type: object + description: >- + The normal response of the operation in case of success. + Depending on the method, this may be the empty object, the resource on which + the operation is performed, or a custom response. required: - - name + - path - done WriteBookStatus: diff --git a/aep/general/0151/lro.proto b/aep/general/0151/lro.proto index 0c29160e..0cfbf5ff 100644 --- a/aep/general/0151/lro.proto +++ b/aep/general/0151/lro.proto @@ -14,19 +14,19 @@ syntax = "proto3"; +import "aep/api/operations.proto"; import "google/api/annotations.proto"; import "google/api/resource.proto"; -import "google/longrunning/operations.proto"; import "google/protobuf/timestamp.proto"; service Library { // Write a book. - rpc WriteBook(WriteBookRequest) returns (google.longrunning.Operation) { + rpc WriteBook(WriteBookRequest) returns (aep.api.Operation) { option (google.api.http) = { post: "/v1/{parent=publishers/*}/books:write" body: "*" }; - option (google.longrunning.operation_info) = { + option (aep.api.operation_info) = { response_type: "WriteBookResponse" metadata_type: "WriteBookMetadata" }; From 0c41cbf460bddd4e286ddb7531ac88cf5c74ede0 Mon Sep 17 00:00:00 2001 From: Richard Frankel Date: Fri, 8 Mar 2024 16:00:10 -0600 Subject: [PATCH 2/4] lint --- aep/general/0151/lro.oas.yaml | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/aep/general/0151/lro.oas.yaml b/aep/general/0151/lro.oas.yaml index 256013a6..93681a1f 100644 --- a/aep/general/0151/lro.oas.yaml +++ b/aep/general/0151/lro.oas.yaml @@ -36,15 +36,16 @@ components: metadata: type: object description: >- - Service-specific metadata associated with the operation. It typically - contains progress information and common metadata such as create time. - Some services might not provide such metadata. Any method that returns a - long-running operation should document the metadata type, if any. + Service-specific metadata associated with the operation. It + typically contains progress information and common metadata such as + create time. Some services might not provide such metadata. Any + method that returns a long-running operation should document the + metadata type, if any. response: type: object description: >- - The normal response of the operation in case of success. - Depending on the method, this may be the empty object, the resource on which + The normal response of the operation in case of success. Depending + on the method, this may be the empty object, the resource on which the operation is performed, or a custom response. required: - path From 604365dc4b1d5bbf7612a33f422b6f8e6329a2ec Mon Sep 17 00:00:00 2001 From: Richard Frankel Date: Fri, 8 Mar 2024 18:07:53 -0600 Subject: [PATCH 3/4] Update creation date --- aep/general/0151/aep.yaml | 1 + 1 file changed, 1 insertion(+) diff --git a/aep/general/0151/aep.yaml b/aep/general/0151/aep.yaml index 4847a70c..611ddced 100644 --- a/aep/general/0151/aep.yaml +++ b/aep/general/0151/aep.yaml @@ -3,6 +3,7 @@ id: 151 state: reviewing slug: long-running-operations created: 2019-07-25 +updated: 2024-03-08 placement: category: standard-methods order: 70 From eb889fb5905fe0c64878c7b72cdca44fc5bcf03f Mon Sep 17 00:00:00 2001 From: Richard Frankel Date: Mon, 11 Mar 2024 11:08:34 -0400 Subject: [PATCH 4/4] Update aep/general/0151/aep.yaml Co-authored-by: Yusuke Tsutsumi --- aep/general/0151/aep.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/aep/general/0151/aep.yaml b/aep/general/0151/aep.yaml index 611ddced..e984ba0a 100644 --- a/aep/general/0151/aep.yaml +++ b/aep/general/0151/aep.yaml @@ -1,6 +1,6 @@ --- id: 151 -state: reviewing +state: approved slug: long-running-operations created: 2019-07-25 updated: 2024-03-08