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

Use Policy<IRestResponse>, AsyncPolicy<IRestResponse> in RetryConfiguration.mustache #7495

Merged
merged 2 commits into from
Sep 29, 2020

Conversation

jeffshantz
Copy link
Contributor

Summary

Use Policy<IRestResponse> and AsyncPolicy<IRestResponse> instead of RetryPolicy<IRestResponse> and AsyncRetryPolicy<IRestResponse> to allow for use of wrapped policies.

Background

In the .NET Core client generator, RetryConfiguration uses the type RetryPolicy<IRestResponse> and AsyncRetryPolicy<IRestResponse> for its properties RetryPolicy and AsyncRetryPolicy, respectively.

This is useful, as it allows one to define a retry policy:

      RetryConfiguration.RetryPolicy = 
        Policy
          .HandleResult<IRestResponse>(r => r.StatusCode == HttpStatusCode.Unauthorized)
          .Retry(retryCount: 3,
                 onRetry: ((result, retryNumber, context) =>
                            {
                              // Fetch new API token here
                              Console.WriteLine("Retrying...");
                            }));

The problem is that this does not allow us to exploit the full utility of Polly. For instance, after retrying 3 times, I might want to log a message or perform some action before giving up. In this case, it would be nice to use Policy.Wrap():

 var retryPolicy =
        Policy
          .HandleResult<IRestResponse>(r => r.StatusCode == HttpStatusCode.Unauthorized)
          .Retry(retryCount: 3,
                 onRetry: ((result, retryNumber, context) =>
                            {
                              // Fetch new API token here
                              Console.WriteLine("Retrying...");
                            }));

      var giveUpPolicy =
        Policy
          .HandleResult<IRestResponse>(r => true)
          .Fallback(fallbackValue: null, onFallback: result =>
          {
            Console.WriteLine("Giving up");
          });

      RetryConfiguration.RetryPolicy = giveUpPolicy.Wrap(retryPolicy);

Solution

This pull request simply changes the type of the RetryPolicy and AsyncRetryPolicy properties to their higher-level base classes in RetryConfiguration such that:

/// <summary>
/// Retry policy
/// </summary>
 public static RetryPolicy<IRestResponse> RetryPolicy { get; set; }

 /// <summary>
/// Async retry policy
/// </summary>
public static AsyncRetryPolicy<IRestResponse> AsyncRetryPolicy { get; set; }

becomes:

/// <summary>
/// Retry policy
/// </summary>
 public static Policy<IRestResponse> RetryPolicy { get; set; }

 /// <summary>
/// Async retry policy
/// </summary>
public static AsyncPolicy<IRestResponse> AsyncRetryPolicy { get; set; }

Then, one can still assign a RetryPolicy as usual, but one can now also use wrapped policies, as above. No other changes are required -- this worked out of the box when I made these type changes.

jeffshantz and others added 2 commits September 23, 2020 14:19
Use Policy<> instead of RetryPolicy<> to allow for use of wrapped policies.
@wing328
Copy link
Member

wing328 commented Sep 24, 2020

Updated samples via 998f5cc. Let's see if that fixes the CI failure.

@wing328
Copy link
Member

wing328 commented Sep 25, 2020

cc @Ramanth (author of the PR adding the retry logic)

cc @mandrean (2017/08) @frankyjuang (2019/09) @shibayan (2020/02)

@jeffshantz
Copy link
Contributor Author

Thanks, @wing328

@wing328
Copy link
Member

wing328 commented Sep 28, 2020

If no further on this PR, I'll merge it tomorrow (Tue).

@wing328 wing328 merged commit b8de51f into OpenAPITools:master Sep 29, 2020
@wing328 wing328 changed the title Update RetryConfiguration.mustache use Policy<IRestResponse>, AsyncPolicy<IRestResponse> in RetryConfiguration.mustache Nov 4, 2020
@wing328 wing328 changed the title use Policy<IRestResponse>, AsyncPolicy<IRestResponse> in RetryConfiguration.mustache Use Policy<IRestResponse>, AsyncPolicy<IRestResponse> in RetryConfiguration.mustache Nov 4, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants