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

Second pass of policy configuration changes #5759

Merged
merged 3 commits into from
Apr 11, 2019

Conversation

pakrym
Copy link
Contributor

@pakrym pakrym commented Apr 10, 2019

Fixes: #5632

  1. Make all policy classes public
  2. Normalizes namespaces to Azure.Base.Pipeline and Azure.Base.Pipeline.Policies
  3. Replaces policy ctor arguments with properties

503, // Service Unavailable
504 // Gateway Timeout
);
public FixedRetryPolicy RetryPolicy { get; set; }
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should the type of this be the base RetryPolicy?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The current thinking is the following consumption model:

var options = new ConfigurationClientOptions();
options.RetryPolicy.Delay = ...;
options.RetryPolicy.MaxRetries = ...;

var client = new ConfigurationClient(options);

It has its downsides but that's the iteration we're on.

@@ -56,15 +56,15 @@ public async Task ComponentNameAndVersionReadFromAssembly()

class CustomRetryPolicy : RetryPolicy
{
protected override bool ShouldRetryResponse(HttpPipelineMessage message, int attempted, out TimeSpan delay)
protected override bool IsRetriableResponse(HttpPipelineMessage message, int attempted, out TimeSpan delay)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Big fan of the name change!

@pakrym pakrym merged commit d137f36 into Azure:master Apr 11, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants