[core-lro] proof-of-concept for https://github.com/Azure/azure-sdk-for-js/issues/32142 #6
Conversation
| @@ -19,6 +19,7 @@ export interface CreateHttpPollerOptions<TResult, TState> { | |||
| resolveOnUnsuccessful?: boolean; | |||
| resourceLocationConfig?: ResourceLocationConfig; | |||
| restoreFrom?: string; | |||
| skipFinalGet?: boolean; | |||
There was a problem hiding this comment.
Name TBD: any suggestions? skipFinalGet seems pretty specific. Other options might be
- withoutResourceLocation
- skipResourceLocation
or something else - feel free to chime in. Thoughts?
There was a problem hiding this comment.
This name could be unclear for someone who is not too familiar with how our pollers work. Could we name it based on impact instead? e.g. noResourceRetrieval. I also wonder if other languages have a similar flag and what name did they choose for it.
There was a problem hiding this comment.
skipResourceLocation or skipGetResource maybe? Would that make sense? I don't have a good domain understanding of the LRO concepts so I am just brainstorming names 👍 feels like resourceLocation is a known term in LROs?
There was a problem hiding this comment.
TBH I even prefer the specific name skipFinalGet here, because for me LRO would be three steps: initial -> polling -> final get(optional) and the main purpose of this option is to always skip the final step. I guess this would be clear with someone who knows LRO.
My concern for ResourceRetrieval is - skipping the final get doesn't mean we will not retrieve the resources, for example maybe the resource is directly returned from the initial request or non-standard LRO returns the resources directly from polling process?
But open to hear feedbacks also.
| @@ -49,6 +51,10 @@ export function createHttpPoller<TResult, TState extends OperationState<TResult> | |||
| init: async () => { | |||
| const response = await lro.sendInitialRequest(); | |||
| const config = inferLroMode(response.rawResponse, resourceLocationConfig); | |||
| if (skipFinalGet && config?.resourceLocation) { | |||
There was a problem hiding this comment.
This will end up moving into inferLroMode so we are not setting config then overriding it. The proof-of-concept just touches as little code as possible
There was a problem hiding this comment.
This change works for me. And I was also thinking of that this could also work in the resumeFrom case.
nit: should we put this logic into inferLroMode?
| @@ -286,6 +287,7 @@ export class KeyVaultBackupClient { | |||
| onResponse: options.onResponse, | |||
| tracingOptions: options.tracingOptions, | |||
| updateIntervalInMs: options.intervalInMs, | |||
| skipFinalGet: true, | |||
There was a problem hiding this comment.
The KeyVault backup client will use this parameter to suppress the final GET
| if ((httpPoller.operationState as any)?.config?.resourceLocation) { | ||
| (httpPoller.operationState as any).config.resourceLocation = undefined; | ||
| } | ||
| // if ((httpPoller.operationState as any)?.config?.resourceLocation) { |
| @@ -110,6 +114,7 @@ export function getLongRunningPoller< | |||
| processResult: (result: unknown) => { | |||
| return processResponseBody(result as TResponse); | |||
| }, | |||
| skipFinalGet: options?.skipFinalGet | |||
There was a problem hiding this comment.
This will need to get codegen'd in all cases
| @@ -19,6 +19,7 @@ export interface CreateHttpPollerOptions<TResult, TState> { | |||
| resolveOnUnsuccessful?: boolean; | |||
| resourceLocationConfig?: ResourceLocationConfig; | |||
| restoreFrom?: string; | |||
| skipFinalGet?: boolean; | |||
There was a problem hiding this comment.
This name could be unclear for someone who is not too familiar with how our pollers work. Could we name it based on impact instead? e.g. noResourceRetrieval. I also wonder if other languages have a similar flag and what name did they choose for it.
maorleger
force-pushed
the
keyvault-admin-typespec
branch
2 times, most recently
from
bef0e52 to
f71b88c
Compare
Wanted to get some early feedback on the approach for resolving Azure#32142 from the core-lro experts. This PR demonstrates the idea behind providing the option to skip the final leg of the LRO where the resource location is used to GET the final resource.
background information: