-
Notifications
You must be signed in to change notification settings - Fork 523
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
Use Polly instead of EnterpriseLibrary #169 #313
Conversation
@@ -21,19 +22,20 @@ public RetryExceptionPolicyFactory(CosmosDataStoreConfiguration configuration) | |||
EnsureArg.IsNotNull(configuration, nameof(configuration)); | |||
|
|||
_maxNumberOfRetries = configuration.RetryOptions.MaxNumberOfRetries; | |||
_minWaitTime = TimeSpan.FromSeconds(Math.Min(RetryStrategy.DefaultMinBackoff.TotalSeconds, configuration.RetryOptions.MaxWaitTimeInSeconds)); | |||
_minWaitTime = TimeSpan.FromSeconds(configuration.RetryOptions.MinWaitTimeInSeconds); | |||
_maxWaitTime = TimeSpan.FromSeconds(configuration.RetryOptions.MaxWaitTimeInSeconds); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like both _minWaitTime
and _maxWaitTime
are unused now. If so these vars can be removed
@@ -98,7 +98,7 @@ public sealed class FhirDataStore : IDataStore, IProvideCapability | |||
_logger.LogDebug($"Upserting {resource.ResourceTypeName}/{resource.ResourceId}, ETag: \"{weakETag?.VersionId}\", AllowCreate: {allowCreate}, KeepHistory: {keepHistory}"); | |||
|
|||
UpsertWithHistoryModel response = await _retryExceptionPolicyFactory.CreateRetryPolicy().ExecuteAsync( | |||
async () => await _upsertWithHistoryProc.Execute( | |||
async (ct) => await _upsertWithHistoryProc.Execute( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unused param. Either remove like below or look at adding/passing through the CancellationToken
in the Execute
function.
async (ct) => await _upsertWithHistoryProc.Execute( | |
async _ => await _upsertWithHistoryProc.Execute( |
_maxWaitTime, | ||
RetryStrategy.DefaultClientBackoff); | ||
var policy = Policy | ||
.Handle<Exception>(RetryExceptionPolicy.IsTransient) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We could probably take more advantage of Polly here and handle DocumentClientException
specifically.
Updating and moving to #369 |
Description
Changed out EnterpriseLibrary transient error retry for Polly
Related issues
Addresses [issue #169].
Testing
All the unit tests pass. Per Polly documentation, there is not way to individual unit test Polly.