Removes Experimental attribute from ResilienceHandler class #5670
Conversation
Removes experimental attribute from ResilienceHandler class
Adds new experimental and stable APIs to the API json file
| @@ -16,7 +16,7 @@ Write-Output "Installing required toolset" | |||
| InitializeDotNetCli -install $true | Out-Null | |||
|
|
|||
| $Project = $PSScriptRoot + "/../eng/Tools/ApiChief/ApiChief.csproj" | |||
| $Command = $PSScriptRoot + "/../artifacts/bin/ApiChief/Debug/net8.0/ApiChief.dll" | |||
| $Command = $PSScriptRoot + "/../artifacts/bin/ApiChief/Debug/net9.0/ApiChief.dll" | |||
There was a problem hiding this comment.
We now build the ApiChief tool for net9.0 that is why I changed this.
| @@ -1,5 +1,5 @@ | |||
| { | |||
| "Name": "Microsoft.Extensions.Http.Resilience, Version=8.0.0.0, Culture=neutral, PublicKeyToken=31bf3856ad364e35", | |||
| "Name": "Microsoft.Extensions.Http.Resilience, Version=9.1.0.0, Culture=neutral, PublicKeyToken=31bf3856ad364e35", | |||
There was a problem hiding this comment.
The API json file hasn't been updated since dotnet/extensions 8.0.0 therefore the changes to this file also include APIs that were changed/updated earlier.
Suppress LA0006 warning causing the build pipeline to fail
| <!-- Disable "Published symbols cannot be deleted to maintain compatibility" warning | ||
| because we have different APIs for different TFMs. --> | ||
| <NoWarn Condition="'$(TargetFramework)' == 'net462'">$(NoWarn);LA0006</NoWarn> | ||
| </PropertyGroup> |
There was a problem hiding this comment.
We have to suppress LA0006 - Published symbols cannot be deleted to maintain compatibility warning. The reason is the following. Together with making the ResilienceHandler class stable, we also made stable protected members SendAsync and Send. These members are inherited from DelegatingHandler and ResilienceHandler overrides them. The Send method is not available on net462 and this causes the build pipeline to fail with the LA0006 warning. To make the build pipeline "green" we have to suppress the LA0006 warning.
There was a problem hiding this comment.
This is suppressing all of the warnings on public symbols though, right? Is there not a way to suppress that single change?
There was a problem hiding this comment.
Also, I'm not very familiar to which analyzer is throwing those errors, but FWIW we do run package validation when we pack projects, which would catch compatibility issues in packages. In this case, net462 and net6.0+ aren't even supposed to be compatible with each other so perhaps the analyzer tooling being used here is not correct.
There was a problem hiding this comment.
This is suppressing all of the warnings on public symbols though, right? Is there not a way to suppress that single change?
Sometime ago I tried to suppress the warning in a source file for a particular change, but it didn't work. Here is another fresher PR #5690 where I tried it again and it didn't work. Unfortunately, we have to suppress the warning for the whole project.
Also, I'm not very familiar to which analyzer is throwing those errors, but FWIW we do run package validation when we pack projects, which would catch compatibility issues in packages. In this case, net462 and net6.0+ aren't even supposed to be compatible with each other so perhaps the analyzer tooling being used here is not correct.
Yes, I agree. I don't understand what exactly the analyzer is catching. Adding a non-abstract method that is available in net6+ and is not on net462 is alright in regards of compatibility, i.e. upgrading from net462 to net6+ will not break. Right?
|
Approving on the assumption the @dotnet/dotnet-extensions-resilience team has done the due diligence for the API review. |
RussKie
added
area-resilience
api-approved
|
Tagging @RussKie and @joperezr as infra-area owners. The PR touches some infra files. Please, review it. Thank you.
Making the API stable was discussed there #4893 (comment). The API looks solid, I think we are good to make it stable. |
The PR removes
Experimentalattribute from ResilienceHandler class.Fixes #5669
Microsoft Reviewers: Open in CodeFlow