Support caller-specified RetryFuncs #4428
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
manicminer
added
enhancement
manicminer
force-pushed
the
support-retry-funcs
branch
from
11e7d2b to
d3ec7f9
Compare
…custom Content-Type header and caller-specified RetryFunc, in addition to the usual marshalled options
…h binary request payloads (e.g. file uploads)
manicminer
force-pushed
the
support-retry-funcs
branch
from
bb19650 to
1e7f8e7
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Changes
TypetoSDKOperationOptionto accommodate additional options that are orthogonal to the existing marshallable options (which become headers, query string arguments, or odata settings).Content-Typeheader viaSDKOperationOptionTypeContentType, useful for file uploads and other scenarios with a customizable request body.RequestRetryFuncviaSDKOperationOptionTypeRequestFunc, more details below.RetryFuncoption to all operations.ContentTypeoption whenever the request body is a binary payload.Background
Currently we have some hardcoded RetryFuncs to handle specific error cases where we want to either safely retry the original request, to elect to not retry a request that would ordinarily be retried (e.g. 429 response), or to provide a custom error message for certain failures. This is already plumbed in and uses go-retryablehttp for backoff and retry handling.
There is already some hardcoded use of RetryFuncs, for example in the Resource Manager Base Client, but individual SDKs can currently also supply a RetryFunc.
With Microsoft Graph, there are lots of edge cases where certain errors need specific handling, and it's not possible to autogenerate these - nor would it be desirable since they would apply for every invocation of an operation absent of any usage context. It's therefore necessary to expose a means of supply a RetryFunc to the caller, so they can be inlined into the AzureAD provider.
This implementation makes it possible for importers to include an optional operation option to allow the caller to specify a RetryFunc. By default this is not added, and doesn't count towards possible options, so this addition will not affect the signatures of any existing operation methods (e.g. in Resource Manager SDKs, existing or future). I've elected to have the MS Graph importer add this option to every operation.
Hamilton has its own equivalent of RetryFuncs, which since it's a handwritten SDK are built into the clients. Some examples: service principals, users, groups, OAuth2 grants, access package resource requests... the list goes on.