Skip to content
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

Add coverage for custom LRO RPC #372

Open
catalinaperalta opened this issue Jul 11, 2023 · 1 comment
Open

Add coverage for custom LRO RPC #372

catalinaperalta opened this issue Jul 11, 2023 · 1 comment

Comments

@catalinaperalta
Copy link
Member

catalinaperalta commented Jul 11, 2023

We should have coverage for LRO RPC operations that follow a similar format to what Health Insights supports.

Link to the TypeSpec: link

Custom LRO RPC template:

@doc("Long running RPC operation template")
@post
op TLongRunningRpcOperation<
  TParams,
  TResponse
> is Azure.Core.Foundations.Operation<
  TParams & RepeatabilityRequestHeaders,
  (Foundations.AcceptedResponse<Foundations.LongRunningStatusLocation<TResponse> &
    Foundations.RetryAfterHeader> &
    RepeatabilityResponseHeaders),
  RepeatabilityRequestHeaders & RepeatabilityResponseHeaders,
  Azure.Core.Foundations.ErrorResponse
>;

Link to operation defintion: link

LRO RPC template usage:

op createJob is TLongRunningRpcOperation<OncoPhenotypeData, OncoPhenotypeResult>;

Health Insights LRO behavior:

  1. Initial POST request to the service with input data in OncoPhenotypeData.
  2. 202 response returned with Operation-Location header, as well as repeatability and retry headers, no body.
  3. Poll at Operation-Location url.
  4. Receive 200 response with final payload OncoPhenotypeResult.
@weidongxu-microsoft
Copy link
Member

weidongxu-microsoft commented Jul 11, 2023

We've had it here #342
But it is not fully reviewed by Mark. We are waiting for issue https://github.com/Azure/typespec-azure/issues/3108 (seems re-routed to Aug)

The template in cadl-ranch lro/rpc/legacy mostly reflect the runtime behavior, except the | TResponse in the last line of

    Foundations.AcceptedResponse<{
      @pollingLocation
      @doc("The location for monitoring the operation state.")
      @TypeSpec.Http.header("Operation-Location")
      operationLocation: TypeSpec.Rest.ResourceLocation<TPollResponse>;
    } & Foundations.RetryAfterHeader> | TResponse

It kind of follow a traditional solution in Swagger, that adding an additional 200 with the "logical result" or "final result" schema. But service never return 200 in that POST.

Removing the | TResponse would cause problem (as "logical result" from helper be empty model; I'd assume the schema of 202 get used instead). This would need to be double checked with Mark on helper method, if we really want to do this.


PS: Mike had a "final-state-schema" in Swagger that will get rid of the extra 200.
https://github.com/Azure/autorest/blob/main/docs/extensions/readme.md#x-ms-long-running-operation-options

But it may take time to get adopted.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants