Skip to content

Instantly share code, notes, and snippets.

@weidongxu-microsoft
Last active April 10, 2023 05:42
Show Gist options
  • Save weidongxu-microsoft/c1194f09e93b82b5656148aa494c2d73 to your computer and use it in GitHub Desktop.
Save weidongxu-microsoft/c1194f09e93b82b5656148aa494c2d73 to your computer and use it in GitHub Desktop.
LRO discussion for Java

Information and runtime behavior of the new design can be found at cadl-ranch PR

Runtime

Major difference in PUT.

  1. Response of the PUT is the Resource object, not the OperationStatus object.
  2. The last GET on Resource URL preferred to be omitted. The Resource object from the PUT is already the final result.

Major difference in POST.

  1. No location header in the response of POST as final resource URL, nor resourceLocation property in PollStatus object. (new guideline discourage POST to create resource; POST should be used for action, not for create)
  2. After POST, only status monitor API be accessed via GET. (as mentioned above, no location, no resourceLocation to point to another URL)
  3. Last OperationStatus object would have result property when sucess, or error property when fail.

No difference in DELETE. LRO on PATCH discouraged.

1. Poller in Java

PUT: PollerFlux<OperationStatus, Resource>

POST: PollerFlux<OperationStatus, Result>

The OperationStatus contains id + status + error properties. The Result from POST is extracted from the result property of the last OperationStatus from status monitor API.

2. Possible solution on polling strategy

New polling strategy

Have a new subclass of OperationResourcePollingStrategy, e.g. OperationLocationPollingStrategy that dedicated to this new LRO design.

When emitter see that the LRO is from TypeSpec Azure.Core, use this OperationLocationPollingStrategy in generated code.

If emitter see the LRO is from service TypeSpec, use existing DefaultPollingStrategy.

Downside

  1. A new class in azure-core (first likely in azure-core-experimental)
  2. If DPG (Swagger) uses this new LRO, we'd need to add polling option to readme.md

Enhance existing polling strategy

It is hard to distinguish the design from the response of initialOperation (as the new design is designed to be mostly backward-compatible at REST API).

Hence, it could be difficult for OperationResourcePollingStrategy, especially for the POST to extract result property (it need to figure out that result property exists, and the value can be converted to Result class, which is a generic type).

Alternatively, we can add an option to existing OperationResourcePollingStrategy, to let it know whether it handles new design. Then it would be very similar to behave as the aforementioned OperationLocationPollingStrategy under this option.

It would be even harder to get DPG (Swagger) to configure this option.

3. Possible solution for OperationStatus model

Add it to azure-core

Again, 1 or 2 new classes in azure-core (OperationStatus and OperationState; OperationState is very similar to LongRunningOperationStatus that exists, so we might able to use LongRunningOperationStatus directly, via some hack in polling strategy).

Ref: TypeSpec models in Azure.Core.Foundations

Avoid new model

However, I don't have good idea about what model we should use for polling result.

PollerFlux<BinaryData, Resource> makes user hard to extract the error (which by default is same model as ResponseError, but can be specialized in Azure.Core).

PollerFlux<ResponseError, Resource> makes the API hard to read, as in most cases, ResponseError won't happen. Probably not PollerFlux<Optional<ResponseError>, Resource> either.


Related PR in azure-core

@weidongxu-microsoft
Copy link
Author

weidongxu-microsoft commented Apr 7, 2023

  1. Agreed on Poller part. An improvement to existing Poller is to be able to getFinalResult in PUT case, before polling complete.
  2. New polling strategy class.
  3. 2 options:
    a. Add id and error to PollResponse. But it may leave the poll result be empty (as PollerResponse has all the info) PollerFlux<Void, Resource>.
    b. Add a e.g. PollResult class that contains id and error. status will not be included as it can be get from PollResponse.

All new classes are first added to core-experimental.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment