-
Notifications
You must be signed in to change notification settings - Fork 1.4k
[core-lro] Proposed LRO Engine design #15949
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
Changes from all commits
d4cc28e
1529790
9667339
d63c5f4
0194cd9
df1eaa9
ea160a8
e723ae6
0fceb82
74cb3fe
49e17f6
26368ea
8ac6a2d
53e8d00
ab68252
3fb9120
4653fd2
6df8c9d
324a2bf
5d52703
828c351
3fe1de0
82d693e
ed0f724
17feb5e
9a689c9
8e775b6
42a94b8
efa6cf2
bc3638e
5abc805
8b8145e
2a36388
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,158 @@ | ||
| # Modular Support for Long-Running Operations | ||
|
|
||
| Long-running operations (LROs) are operations that the service _could_ take a long time to finish processing and they follow a common convention: | ||
|
|
||
| - the customer first send an initiation request to the service, which in turn sends back a response, from which the customer can learn how to poll for the status of the operation, if it has not been completed already, | ||
| - using their learnings, the customer polls the status of the operation until it is done, | ||
| - again, using their learnings, the customer can now get the desired result of the operation once its status says it is done. | ||
|
|
||
| Ideally, we can write an algorithm that implements this convention once and use it in all Azure clients for LRO APIs, however, in reality, this convention is implemented differently across Azure services. The good news is that the TypeScript Autorest extension is AFAIK able to generate code that implements those different ones, but this implementation has a couple of limitations: | ||
|
|
||
| 1. it is located in a few files that the code generator copies into every generated package that has LROs. So if in the future there is a bug fix needed in the LRO logic, the package has to be regenerated with the fix. | ||
| 2. it works only with clients that use `@azure/core-client`, so clients that use `@azure-rest/core-client` or `@azure/core-http` can not use this implementation as-is. | ||
|
|
||
| To fix limitation #1, the most straightforward thing to do is to move those files into `@azure/core-lro`, but without fixing limitation #2 first, `@azure/core-lro` will have to depend on `@azure/core-client` in this case which will force clients that depend on `@azure/core-lro` but not necessarily depend on `@azure/core-client` to transitively depend on the latter, posing concerns about unnecessarily larger bundle sizes. | ||
|
|
||
| This document presents a design that fixes limitation #2 and naturally fixes limitation #1 too. | ||
|
|
||
| ## Things to know before reading | ||
|
|
||
| - Some details not related to the high-level concept are not illustrated; the scope of this is limited to the high-level shape and paradigms for the feature area. | ||
|
|
||
| ## Terminology | ||
|
|
||
| - **Azure Async Operation**, **Body**, and **Location** are names for the LRO implementations currently supported in the TypeScript Autorest extension. They vary in how to calculate the path to poll from, the algorithm for detecting whether the operation has finished, and the location to retrieve the desired results from. Currently, these pieces of information can be calculated from the response received after sending the initiation request. | ||
|
|
||
| ## Why this is needed | ||
|
|
||
| The China team is currently waiting for fixing limitation #1 which they regard as a blocker for GAing the TypeScript Autorest extension. Furthermore, having this LRO implementation being part of `@azure/core-lro` and not tied to `@azure/core-client` will significantly help streamline the underway effort to add convenience helpers for LROs in `@azure-rest` clients. | ||
|
|
||
| ## Proposed design | ||
|
|
||
| This document presents a design of an LRO engine to be part of `@azure/core-lro` and could be used by any client regardless of how it is implemented. Furthermore, specific implementations of the engine are also provided to be auto-generated by Autorest. | ||
|
|
||
| The design consists of three main pieces: | ||
|
|
||
| - an interface, named `LongRunningOperation<T>` which groups various primitives needed to implement LROs | ||
| - a class, named `LroEngine`, that implements the LRO engine and its constructor takes as input an object that implements `LongRunningOperation<T>` | ||
| - a class that implement `LongRunningOperation<T>` that works with clients that use either `@azure/core-http` and `@azure/core-client`. @joheredi also created one for `@azure-rest/core-client` in https://github.com/Azure/azure-sdk-for-js/pull/15898 | ||
|
|
||
| ### `LongRunningOperation<T>` | ||
|
|
||
| This interface contains two methods: **sendInitialRequest** and **sendPollRequest**. I propose to make this interface exported by `@azure/core-lro`. | ||
|
|
||
| #### `sendInitialRequest` | ||
|
|
||
| This method should be implemented to send the initial request to start the operation and it has the following signature: | ||
|
|
||
| ```ts | ||
| sendInitialRequest: () => Promise<LroResponse<T>> | ||
| ``` | ||
|
|
||
| The method does not take the path or the HTTP request method as parameters because they're members of the interface since they're needed to control many aspects of subsequent polling. This is how this method can be implemented: | ||
|
|
||
| ```ts | ||
| public async sendInitialRequest(): Promise<LroResponse<T>> { | ||
| return this.sendOperation(this.args, this.spec); // the class will have sendOperation, args, and spec as private fields | ||
| } | ||
| ``` | ||
|
|
||
| #### `sendPollRequest` | ||
|
|
||
| This method should be implemented to send a polling (GET) request, a request the service should respond to with the current status of the operation, and it has the following signature: | ||
|
|
||
| ```ts | ||
| sendPollRequest: (path: string) => Promise<LroResponse<T>>; | ||
| ``` | ||
|
|
||
| This method takes the polling path as input and here is what a simplified implementation would look like: | ||
|
|
||
| ```ts | ||
| public async sendPollRequest(path: string): Promise<LroResponse<T>> { | ||
| return this.sendOperationFn(this.args, { // the class will have sendOperation, args, and spec as private fields | ||
| ...this.spec, | ||
| path, | ||
| httpMethod: "GET") | ||
| }); | ||
| } | ||
| ``` | ||
|
|
||
| ### `LroEngine` | ||
|
|
||
| This class implements the `PollerLike` interface and does the heavy lifting for LROs. I propose to make this class also exported by `@azure/core-lro`. This class has the following type signature: | ||
|
|
||
| ```ts | ||
| class LroEngine<TResult, TState extends PollOperationState<TResult>> extends Poller<TState, TResult> | ||
| ``` | ||
|
|
||
| The class also has the following constructor: | ||
|
|
||
| ```ts | ||
| constructor(lro: LongRunningOperation<TResult>, options?: LroEngineOptions); | ||
| ``` | ||
|
|
||
| Currently `options` have `intervalInMs` to control the polling interval, `resumeFrom` to enable resuming from a serialized state, and `lroResourceLocationConfig` which could determine where to find the results of the LRO after the operation is finished. Typically, Autorest figures out the value for `LroResourceLocationConfig` from the `x-ms-long-running-operation-options` swagger extension. If there are new arguments to be added to the class, they could be added to the options type. | ||
|
|
||
| ### [`LroImpl`](https://github.com/deyaaeldeen/autorest.typescript/blob/lro-simplify/src/lroImpl.ts) | ||
|
|
||
| This class implements the `LongRunningOperation<T>` interface and I propose to make it auto-generated by Autorest. `LroImpl` needs access to a few pieces: operation specification and operation arguments and a primitive function that can take them as input to send a request and converts the received response into one of type `LroResponse<T>` which has both the flattened and the raw responses. | ||
|
|
||
| ## Usage examples | ||
|
|
||
| ### Create an object of `LroImpl` | ||
|
|
||
| ```ts | ||
| const directSendOperation = async ( | ||
| args: OperationArguments, | ||
| spec: OperationSpec | ||
| ): Promise<unknown> => { | ||
| return this.client.sendOperationRequest(args, spec); | ||
deyaaeldeen marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| }; | ||
| const sendOperation = async ( | ||
| args: OperationArguments, | ||
| spec: OperationSpec | ||
| ) => { | ||
| let currentRawResponse: FullOperationResponse | undefined = undefined; | ||
| const providedCallback = args.options?.onResponse; | ||
| const callback: RawResponseCallback = ( | ||
| rawResponse: FullOperationResponse, | ||
| flatResponse: unknown | ||
| ) => { | ||
| currentRawResponse = rawResponse; | ||
| providedCallback?.(rawResponse, flatResponse); | ||
| }; | ||
| const updatedArgs = { | ||
| ...args, | ||
| options: { | ||
| ...args.options, | ||
| onResponse: callback | ||
| } | ||
| }; | ||
| const flatResponse = await directSendOperation(updatedArgs, spec); | ||
| return { | ||
| flatResponse, | ||
| rawResponse: { | ||
| statusCode: currentRawResponse!.status, | ||
| body: currentRawResponse!.parsedBody, | ||
| headers: currentRawResponse!.headers.toJSON() | ||
| } | ||
| }; | ||
| }; | ||
|
|
||
| const lro = new LroImpl( | ||
| sendOperation, | ||
| { options }, // arguments are just the operation options | ||
| spec | ||
| ); | ||
| ``` | ||
|
|
||
| ### Using `LroEngine` | ||
|
|
||
| ```ts | ||
| const pollerEngine = new LroEngine(lro, { intervalInMs: 2000 }); // lro was instantiated in the previous section | ||
| const result = pollerEngine.pollUntilDone(); | ||
| ``` | ||
|
|
||
| ## Testing | ||
|
|
||
| We have [extensive test suite for LROs](https://github.com/Azure/autorest.typescript/blob/main/test/integration/lro.spec.ts) in the TypeScript code generator repo. I both added those tests here and re-implemented the [lro routes](https://github.com/Azure/autorest.testserver/blob/main/legacy/routes/lros.js) in the Autorest test server. For this to work, I created a [fairly low-level instantiation for `LongRunningOperation<T>` with just `@azure/core-rest-pipeline`](https://github.com/deyaaeldeen/azure-sdk-for-js/blob/lro-design/sdk/core/core-lro/test/utils/coreRestPipelineLro.ts). | ||
deyaaeldeen marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -3,3 +3,4 @@ | |
|
|
||
| export * from "./pollOperation"; | ||
| export * from "./poller"; | ||
| export * from "./lroEngine"; | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,80 @@ | ||
| // Copyright (c) Microsoft Corporation. | ||
| // Licensed under the MIT license. | ||
|
|
||
| import { | ||
| failureStates, | ||
| LroResourceLocationConfig, | ||
| LongRunningOperation, | ||
| LroBody, | ||
| LroResponse, | ||
| LroStatus, | ||
| RawResponse, | ||
| successStates | ||
| } from "./models"; | ||
| import { isUnexpectedPollingResponse } from "./requestUtils"; | ||
|
|
||
| function getResponseStatus(rawResponse: RawResponse): string { | ||
| const { status } = (rawResponse.body as LroBody) ?? {}; | ||
| return status?.toLowerCase() ?? "succeeded"; | ||
| } | ||
|
|
||
| function isAzureAsyncPollingDone(rawResponse: RawResponse): boolean { | ||
| const state = getResponseStatus(rawResponse); | ||
| if (isUnexpectedPollingResponse(rawResponse) || failureStates.includes(state)) { | ||
| throw new Error(`The long running operation has failed. The provisioning state: ${state}.`); | ||
| } | ||
| return successStates.includes(state); | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm a little curious about why failure is exceptional instead of being viewed as a final state, like success. Is failure never expected to happen normally, so this method is really just disambiguating between success and in progress?
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The code gen crew agreed on throwing if the operation reached a failure state and this function just implements this expectation. I think it is mostly because other languages started this way so JS has to be consistent with them. To me, I think this is not the best thing to do because all the customer gets is an exception message which I think it makes rehydration harder. |
||
| } | ||
|
|
||
| /** | ||
| * Sends a request to the URI of the provisioned resource if needed. | ||
| */ | ||
| async function sendFinalRequest<TResult>( | ||
deyaaeldeen marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| lro: LongRunningOperation<TResult>, | ||
| resourceLocation: string, | ||
| lroResourceLocationConfig?: LroResourceLocationConfig | ||
| ): Promise<LroResponse<TResult> | undefined> { | ||
| switch (lroResourceLocationConfig) { | ||
| case "original-uri": | ||
| return lro.sendPollRequest(lro.requestPath); | ||
| case "azure-async-operation": | ||
| return undefined; | ||
| case "location": | ||
| default: | ||
| return lro.sendPollRequest(resourceLocation ?? lro.requestPath); | ||
| } | ||
| } | ||
|
|
||
| export function processAzureAsyncOperationResult<TResult>( | ||
| lro: LongRunningOperation<TResult>, | ||
| resourceLocation?: string, | ||
| lroResourceLocationConfig?: LroResourceLocationConfig | ||
| ): (response: LroResponse<TResult>) => LroStatus<TResult> { | ||
| return (response: LroResponse<TResult>): LroStatus<TResult> => { | ||
| if (isAzureAsyncPollingDone(response.rawResponse)) { | ||
| if (resourceLocation === undefined) { | ||
| return { ...response, done: true }; | ||
| } else { | ||
| return { | ||
| ...response, | ||
| done: false, | ||
| next: async () => { | ||
| const finalResponse = await sendFinalRequest( | ||
| lro, | ||
| resourceLocation, | ||
| lroResourceLocationConfig | ||
| ); | ||
| return { | ||
| ...(finalResponse ?? response), | ||
| done: true | ||
| }; | ||
| } | ||
| }; | ||
| } | ||
| } | ||
| return { | ||
| ...response, | ||
| done: false | ||
| }; | ||
| }; | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,39 @@ | ||
| // Copyright (c) Microsoft Corporation. | ||
| // Licensed under the MIT license. | ||
|
|
||
| import { | ||
| failureStates, | ||
| LroBody, | ||
| LroResponse, | ||
| LroStatus, | ||
| RawResponse, | ||
| successStates | ||
| } from "./models"; | ||
| import { isUnexpectedPollingResponse } from "./requestUtils"; | ||
|
|
||
| function getProvisioningState(rawResponse: RawResponse): string { | ||
| const { properties, provisioningState } = (rawResponse.body as LroBody) ?? {}; | ||
| const state: string | undefined = properties?.provisioningState ?? provisioningState; | ||
| return state?.toLowerCase() ?? "succeeded"; | ||
| } | ||
|
|
||
| export function isBodyPollingDone(rawResponse: RawResponse): boolean { | ||
| const state = getProvisioningState(rawResponse); | ||
| if (isUnexpectedPollingResponse(rawResponse) || failureStates.includes(state)) { | ||
| throw new Error(`The long running operation has failed. The provisioning state: ${state}.`); | ||
| } | ||
| return successStates.includes(state); | ||
| } | ||
|
|
||
| /** | ||
| * Creates a polling strategy based on BodyPolling which uses the provisioning state | ||
| * from the result to determine the current operation state | ||
| */ | ||
| export function processBodyPollingOperationResult<TResult>( | ||
| response: LroResponse<TResult> | ||
| ): LroStatus<TResult> { | ||
| return { | ||
| ...response, | ||
| done: isBodyPollingDone(response.rawResponse) | ||
| }; | ||
| } |
Uh oh!
There was an error while loading. Please reload this page.