-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
#2078 V7 polly syntax not longer supported #2079
Conversation
Next time, could you create feature branch in your personal folder like: |
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.
Generally, there's nothing to review.
However, I'm concerned about the tests that were removed. My heart is broken 💔.
Perhaps some of them are still valuable?
Important changes in version `23.2`_: [#f3]_ | ||
|
||
- With `Polly`_ version 8+, the ``ExceptionsAllowedBeforeBreaking`` value must be equal to or greater than **2**! |
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.
I believe these lines can be retained and relocated to an earlier position.
@@ -34,7 +34,7 @@ | |||
<PackageReference Include="StyleCop.Analyzers" Version="1.2.0-beta.507"> | |||
<PrivateAssets>all</PrivateAssets> | |||
</PackageReference> | |||
<PackageReference Include="Polly" Version="8.3.1" /> | |||
<PackageReference Include="Polly" Version="8.4.0" /> |
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.
Right! Good to bump the version!
@@ -112,96 +111,4 @@ public static IOcelotBuilder AddPolly(this IOcelotBuilder builder) | |||
/// <returns>A <see cref="DelegatingHandler"/> object, but concrete type is the <see cref="PollyResiliencePipelineDelegatingHandler"/> class.</returns> | |||
private static DelegatingHandler GetDelegatingHandler(DownstreamRoute route, IHttpContextAccessor contextAccessor, IOcelotLoggerFactory loggerFactory) | |||
=> new PollyResiliencePipelineDelegatingHandler(route, contextAccessor, loggerFactory); | |||
|
|||
#region Obsolete extensions will be removed in future version |
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.
I love when regions are removed 😃
@@ -75,7 +75,7 @@ | |||
<PackageReference Include="CacheManager.Core" Version="2.0.0-beta-1629" /> | |||
<PackageReference Include="CacheManager.Microsoft.Extensions.Configuration" Version="2.0.0-beta-1629" /> | |||
<PackageReference Include="CacheManager.Microsoft.Extensions.Logging" Version="2.0.0-beta-1629" /> | |||
<PackageReference Include="Polly" Version="8.3.1" /> | |||
<PackageReference Include="Polly" Version="8.4.0" /> |
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.
I'm not sure but the reference should be auto-restored because of ref to Ocelot
, right?
I believe we could deliver it as a part of 23.3 |
Consolidate all notes into the Notes section
Removal of AddPollyV7 and all subsequent code.
Removal of AddPollyV7 tests