[AppConfig] allow setting updateIntervalInMs in LRO options
#27200
Conversation
also add a `testPollingOptions` in recorder to help testing in playback mode.
jeremymeng
requested review from
HarshaNalluru,
timovv and
minhanh-phan
as code owners
github-actions
bot
added
the
App Configuration
|
I am a little surprised that this LRO option doesn't have our standard options? |
|
API change check APIView has identified API level changes in this PR and created following API reviews. |
| * Polling options that don't wait in playback mode. | ||
| */ | ||
| export const testPollingOptions = { | ||
| updateIntervalInMs: isPlaybackMode() ? 0 : undefined, |
There was a problem hiding this comment.
NIT
we can import the type from core-lro, so that they are in sync always.
There was a problem hiding this comment.
core-lro's LroEngineOptions has a different property intervalInMs, which means some other packages needs a different testing option...
There was a problem hiding this comment.
we can add intervalInMs: isPlaybackMode() ? 0 : undefined, here too when needed in the future, it should be compatible with both.
| /** | ||
| * The amount of time to wait (in milliseconds) between subsequent requests relating to the same operation. | ||
| */ | ||
| updateIntervalInMs?: number; |
There was a problem hiding this comment.
beginCreateSnapshot() in the generated code takes this CreateSnapshotOptionalParams as the options bag, unsure how this fell off.
export interface CreateSnapshotOptionalParams
extends coreClient.OperationOptions {
/** Delay to wait until next poll, in milliseconds. */
updateIntervalInMs?: number;
/** A serialized poller which can be used to resume an existing paused Long-Running-Operation. */
resumeFrom?: string;
}
updateIntervalInMs makes sense.
resumeFrom - I'm not sure about. Should we skip this? Who pauses the operation? service pauses it and we resume it? How is this typically used?
There was a problem hiding this comment.
Maybe it is supported by codegen but I have not tested resumeFrom so didn't add it this time.
There was a problem hiding this comment.
Let's merge the PR anyway since it is unrelated.
@deyaaeldeen if you can shed some light on resumeFrom... we can take it in another PR
There was a problem hiding this comment.
I think the idea is that you serialize the current poller state to a string, then later you can resume by rehydrating the state
There was a problem hiding this comment.
resumeFrom is a string-representation of the poller's state. For instance, the client can decide to serialize the poller state and send it over somewhere else and do polling there instead. I personally don't like it too much because it is an option and you still have to provide required inputs to the method anyway?
There was a problem hiding this comment.
Also, it is supported in all core-lro's pollers.
also add a
testPollingOptionsin recorder to help testing in playback mode.