Revert "Update to latest Azure.Core package with Azure.Identity types"#4236
Revert "Update to latest Azure.Core package with Azure.Identity types"#4236cheenamalhotra merged 1 commit intomainfrom
Conversation
There was a problem hiding this comment.
Pull request overview
Reverts the dependency updates from #4200 to keep Azure-related package versions aligned with the 7.1.0-preview1 plan (deferring newer Azure/Abstractions updates to preview2).
Changes:
- Adjusts test projects to reference
Azure.Identityinstead ofAzure.Core. - Updates centralized dependency versions (downgrades
Azure.Core/Microsoft.Identity.Client, addsAzure.Identity). - Updates doc/sample projects and AzureAuthentication app docs/package pins (including moving several defaults to preview build versions).
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| src/Microsoft.Data.SqlClient/tests/ManualTests/Microsoft.Data.SqlClient.ManualTests.csproj | Swaps Azure package reference in ManualTests (net462 + netcore item groups). |
| src/Microsoft.Data.SqlClient/tests/FunctionalTests/Microsoft.Data.SqlClient.FunctionalTests.csproj | Swaps Azure package reference in FunctionalTests (net462 + netcore item groups). |
| src/Microsoft.Data.SqlClient/tests/Common/Microsoft.Data.SqlClient.TestCommon.csproj | Swaps Azure package reference in TestCommon (net462 + netcore item groups). |
| src/Microsoft.Data.SqlClient.Extensions/Azure/src/Azure.csproj | Adds Azure.Identity dependency to the Extensions.Azure package project. |
| doc/samples/Microsoft.Data.SqlClient.Samples.csproj | Adds Azure.Core reference alongside Azure.Identity for samples. |
| doc/apps/AzureAuthentication/README.md | Updates default/example package version strings in documentation. |
| doc/apps/AzureAuthentication/Directory.Packages.props | Changes default package versions and package version pins for the sample app. |
| doc/Directory.Packages.props | Pins doc projects to specific preview versions of SqlClient/AKV/Azure extensions packages. |
| Directory.Packages.props | Downgrades Azure.Core/MSAL versions and introduces a central version for Azure.Identity. |
| @@ -355,7 +355,7 @@ | |||
| <ItemGroup Condition="'$(TargetFramework)' == 'net462'"> | |||
| <Reference Include="System.Transactions" /> | |||
|
|
|||
There was a problem hiding this comment.
ManualTests uses Azure.Core types (e.g., TokenCredential/AccessToken), but this project no longer references Azure.Core directly. Because CentralPackageTransitivePinningEnabled is disabled at the repo level, relying on Azure.Identity (or other packages) to bring in Azure.Core transitively means the intended centrally pinned Azure.Core version may not be applied consistently. Consider adding Azure.Core back as an explicit PackageReference alongside Azure.Identity for both TFMs.
| <PackageReference Include="Azure.Core" /> |
| <ItemGroup Condition="'$(TargetFramework)' == 'net462'"> | ||
| <PackageReference Include="Azure.Core" /> | ||
| <PackageReference Include="Azure.Identity" /> | ||
| <PackageReference Include="Azure.Security.KeyVault.Keys" /> |
There was a problem hiding this comment.
This shared test helper library uses Azure.Core types (e.g., TokenCredential), but Azure.Core is no longer referenced directly. Since the repo disables CentralPackageTransitivePinningEnabled, Azure.Core version selection can drift when only pulled transitively. Add an explicit Azure.Core PackageReference alongside Azure.Identity to ensure consistent restore/build behavior.
| <Reference Include="System.Transactions" /> | ||
|
|
||
| <PackageReference Include="Azure.Core" /> | ||
| <PackageReference Include="Azure.Identity" /> |
There was a problem hiding this comment.
This project now references Azure.Identity, but there are no usages of Azure.Identity (or Azure.Core) types in FunctionalTests sources. If it’s no longer needed, removing this PackageReference will reduce restore time and dependency surface; if it’s required indirectly, it would be better documented via the project/package that actually uses it.
| <PackageReference Include="Azure.Identity" /> |
| -p:SqlClientVersion=7.1.0.preview1 | ||
| -p:AkvProviderVersion=7.0.0 | ||
| -p:AzureVersion=1.1.0-preview1 | ||
| -p:SqlClientVersion=7.0.0.preview4 |
There was a problem hiding this comment.
The command-line example uses an invalid version format (7.0.0.preview4 with a dot). NuGet package versions use hyphenated prerelease labels (e.g., 7.0.0-preview4), so this copy/paste example will fail restore/build.
| -p:SqlClientVersion=7.0.0.preview4 | |
| -p:SqlClientVersion=7.0.0-preview4 |
|
|
||
| ```bash | ||
| dotnet run -p:SqlClientVersion=7.1.0-preview1 -p:AkvProviderVersion=7.1.0-preview1 -p:AzureVersion=1.0.0 -- -c "<connection string>" | ||
| dotnet run -p:SqlClientVersion=7.0.0-preview1 -p:AkvProviderVersion=7.0.0-preview1 -p:AzureVersion=1.0.0-preview1 -- -c "<connection string>" |
There was a problem hiding this comment.
This example also uses a mismatched/likely invalid version string (7.0.0-preview1) compared to the earlier defaults and examples. Please align the example values with the intended package versions (and ensure the versions exist on the configured feeds) to avoid copy/paste failures.
| dotnet run -p:SqlClientVersion=7.0.0-preview1 -p:AkvProviderVersion=7.0.0-preview1 -p:AzureVersion=1.0.0-preview1 -- -c "<connection string>" | |
| dotnet run -p:SqlClientVersion=7.0.0-preview4 -p:AkvProviderVersion=7.0.0-preview4 -p:AzureVersion=1.0.0-preview1 -- -c "<connection string>" |
| <PropertyGroup> | ||
| <!-- Use SqlClient 7.0.0 if no version was specified. --> | ||
| <SqlClientVersion>7.0.0</SqlClientVersion> | ||
| <!-- Use SqlClient 7.0.0 Preview 4 if no version was specified. --> | ||
| <SqlClientVersion>7.0.0-preview4.26064.3</SqlClientVersion> | ||
|
|
||
| <!-- Use AKV Provider 7.0.0 if no version was specified. --> | ||
| <AkvProviderVersion>7.0.0</AkvProviderVersion> | ||
| <!-- Use AKV Provider 7.0.0 Preview 1 if no version was specified. --> | ||
| <AkvProviderVersion>7.0.0-preview1.26064.3</AkvProviderVersion> | ||
| </PropertyGroup> |
There was a problem hiding this comment.
The default package versions are now pinned to very specific preview builds (with build metadata). Given the app’s NuGet.config points at nuget.org plus a local folder, these defaults will fail restore unless the exact packages are present locally. Consider keeping defaults on published versions and using -p: overrides for local preview builds, or update the documentation/NuGet.config expectations accordingly.
| <!-- Reference SqlClient package versions that support our needs. --> | ||
| <PackageVersion Include="Microsoft.Data.SqlClient" Version="7.0.0" /> | ||
| <PackageVersion Include="Microsoft.Data.SqlClient.AlwaysEncrypted.AzureKeyVaultProvider" Version="7.0.0" /> | ||
| <PackageVersion Include="Microsoft.Data.SqlClient.Extensions.Azure" Version="1.0.0" /> | ||
| <PackageVersion Include="Microsoft.Data.SqlClient" Version="7.0.0-preview4.26064.3" /> | ||
| <PackageVersion Include="Microsoft.Data.SqlClient.AlwaysEncrypted.AzureKeyVaultProvider" Version="7.0.0-preview1.26064.3" /> | ||
| <PackageVersion Include="Microsoft.Data.SqlClient.Extensions.Azure" Version="1.0.0-preview1.26064.3" /> |
There was a problem hiding this comment.
This doc-wide package pinning now targets specific preview build versions of SqlClient/AkvProvider/Azure extensions. Unless those exact versions are guaranteed to exist in the configured feeds (governed + local), restoring/building the doc samples can fail. Please confirm this is intentional for 7.1.0-preview1, or revert these to published/stable versions and rely on local overrides when needed.
| <ItemGroup Condition="'$(TargetFramework)' != 'net462'"> | ||
| <PackageReference Include="Azure.Core" /> | ||
| <PackageReference Include="Azure.Identity" /> | ||
| <PackageReference Include="Azure.Security.KeyVault.Keys" /> |
There was a problem hiding this comment.
Microsoft.Data.SqlClient.TestCommon doesn’t appear to use any Azure.Identity types (only Azure.Core + KeyVault Keys). If that’s still the case, consider removing Azure.Identity here and referencing Azure.Core explicitly instead, to avoid an unused dependency.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #4236 +/- ##
==========================================
- Coverage 65.92% 65.73% -0.19%
==========================================
Files 276 272 -4
Lines 42962 65813 +22851
==========================================
+ Hits 28322 43264 +14942
- Misses 14640 22549 +7909
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Reverts #4200 for 7.1.0-preview1 - as we do not plan to release new versions of Azure and Abstractions packages right now.
We will take these changes later for 7.1.0-preview2 milestone.
There are dependency leak issues that need to be resolved in 7.0.x before new versions of Azure and Abstractions can be released.