Skip to content

Conversation

@paulmedynski
Copy link
Contributor

@paulmedynski paulmedynski commented Jan 20, 2026

Description

This PR contains the new Abstractions and Azure package files:

  • Added the new Abstractions and Azure source files and associated pipeline files.
  • Setup build.proj targets and default version numbers.
  • Standardized project CLS compliance.

PR series:

Testing

  • Normal PR/CI pipeline runs will validate most changes.
  • Manual inspection of pipeline logs and artifacts will confirm some of the fiddly parts.

- Added the new Abstractions and Azure source files and associated pipeline files.
- Setup build.proj targets and default version numbers.
- Standardized project CLS compliance.
Copilot AI review requested due to automatic review settings January 20, 2026 19:52
@paulmedynski paulmedynski added the Area\Azure Connectivity Use this to tag issues that are related to Azure connectivity. label Jan 20, 2026
@paulmedynski paulmedynski added this to the 7.0.0-preview4 milestone Jan 20, 2026
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This pull request is Step 2 of the Azure split effort, adding new Abstractions and Azure extension packages to decouple Azure dependencies from the main Microsoft.Data.SqlClient package. The PR introduces two new packages with their source code, tests, documentation, and CI/CD pipeline configurations while standardizing CLS compliance across projects.

Changes:

  • Added new Microsoft.Data.SqlClient.Extensions.Abstractions package with base types for authentication and related abstractions
  • Added new Microsoft.Data.SqlClient.Extensions.Azure package containing Azure authentication implementations moved from MDS
  • Standardized CLS compliance by using AssemblyAttribute declarations instead of embedded assembly attributes
  • Added comprehensive CI/CD pipeline support for building and testing both new packages across multiple platforms

Reviewed changes

Copilot reviewed 69 out of 69 changed files in this pull request and generated 13 comments.

Show a summary per file
File Description
tools/targets/GenerateThisAssemblyCs.targets Simplified to unconditionally generate ThisAssembly.cs with configurable namespace
tools/props/Versions.props Added imports for Abstractions and Azure version properties
src/Microsoft.Data.SqlClient/tests/tools/TDS/**/*.csproj Removed ClsCompliant property (standardization)
src/Microsoft.Data.SqlClient/**/Microsoft.Data.SqlClient.csproj Added CLS compliance via AssemblyAttribute
src/Microsoft.Data.SqlClient.sln Added folder structure and projects for Abstractions and Azure packages
src/Microsoft.Data.SqlClient.Extensions/Abstractions/** Complete Abstractions package with source, tests, and documentation
src/Microsoft.Data.SqlClient.Extensions/Azure/** Complete Azure package with source, tests, and documentation
eng/pipelines/** New CI/CD stages, jobs, and variables for building/testing new packages
build.proj Added targets for building Abstractions and Azure packages
Directory.Packages.props Added package version management for new packages
BUILDGUIDE.md Updated documentation with new build targets

parameters:
referenceType: Package
buildConfiguration: Release
cleanFirst: true
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 build.proj Clean target removes all generated files, including the packages/ directory. This is the behaviour we want from a Clean. However, removing packages/ causes restores to fail when we're using our local NuGet.config. This is mostly done in the pipelines, so we must remove the cleans here, which weren't doing anything useful anyway.

# See the LICENSE file in the project root for more information. #
#################################################################################

# This file is only included in MDS OneBranch Official pipelines.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

There are too many YAML variables and files. This will at least make it easier to reason about which variables affect which pipelines.


# Build the Abstractions package, and publish it to the pipeline artifacts
# under the given artifact name.
- template: /eng/pipelines/stages/build-abstractions-package-ci-stage.yml@self
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This PR adds the new Abstractions and Azure packages to the pipelines, but MDS and its tests don't use them yet.

// NOTE: The current Microsoft.VSDesigner editor attributes are implemented for System.Data.SqlClient, and are not publicly available.
// New attributes that are designed to work with Microsoft.Data.SqlClient and are publicly documented should be included in future.

[assembly: System.CLSCompliant(true)]
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We were setting CLS compliance several ways, so I standardized on the project approach.

<AssemblyName>Microsoft.SqlServer.TDS.Servers</AssemblyName>
<ProjectGuid>{978063D3-FBB5-4E10-8C45-67C90BE1B931}</ProjectGuid>
<TargetFrameworks>netstandard2.0</TargetFrameworks>
<ClsCompliant>false</ClsCompliant>
Copy link
Contributor Author

Choose a reason for hiding this comment

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

There is no such MSBuild property, so this wasn't doing anything.

EndProject
Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "Microsoft.Data.SqlClient.TestCommon", "Microsoft.Data.SqlClient\tests\Common\Microsoft.Data.SqlClient.TestCommon.csproj", "{3FF03FA9-E3C3-49E3-9DCB-C703A5B0278B}"
EndProject
Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "Microsoft.Data.SqlClient.Extensions", "Microsoft.Data.SqlClient.Extensions", "{19F1F1E5-3013-7660-661A-2A15F7D606C1}"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I used dotnet sln add to add the new projects.

<ThisAssemblyNamespace Condition="'$(ThisAssemblyNamespace)' == ''">System</ThisAssemblyNamespace>

<ThisAssemblyCsContents>
[assembly: System.CLSCompliant(true)]
Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's was too sneaky using this target to inject CLS compliance. Moving it to all of the relevant project files makes it clear and obvious.

</ThisAssemblyCsContents>
</PropertyGroup>

<!-- Get the last version number we built with (if any) -->
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This was over-engineered - there's no need to attempt to avoid generating this file on every build.

<PropertyGroup>

<!-- Put the ThisAssembly class in the System namespace by default. -->
<ThisAssemblyNamespace Condition="'$(ThisAssemblyNamespace)' == ''">System</ThisAssemblyNamespace>
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is now shared by 3 projects, and they all need to put this class in a unique namespace.

- Added ThisAssembly generation to Abstractions.
- Fixed branch wildcards in PR pipeline and CodeQL workflow triggers.
Copilot AI review requested due to automatic review settings January 21, 2026 11:10
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 72 out of 72 changed files in this pull request and generated no new comments.

- Inhibited some Azure tests that won't pass yet.
- Addressed Copilot comments in the PR.
@codecov
Copy link

codecov bot commented Jan 21, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 69.92%. Comparing base (b5fafcb) to head (e4fdb36).

Additional details and impacted files
@@                   Coverage Diff                    @@
##           dev/paul/azure/step1    #3904      +/-   ##
========================================================
- Coverage                 69.96%   69.92%   -0.04%     
========================================================
  Files                       263      263              
  Lines                     66190    66190              
========================================================
- Hits                      46307    46285      -22     
- Misses                    19883    19905      +22     
Flag Coverage Δ
netcore 69.91% <ø> (-0.05%) ⬇️
netfx 69.17% <ø> (-0.08%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@paulmedynski paulmedynski marked this pull request as ready for review January 21, 2026 17:47
@paulmedynski paulmedynski requested a review from a team as a code owner January 21, 2026 17:47
Copilot AI review requested due to automatic review settings January 21, 2026 17:47
@paulmedynski paulmedynski changed the title [DRAFT] Azure Split - Step 2 - New Files Azure Split - Step 2 - New Files Jan 21, 2026
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 72 out of 72 changed files in this pull request and generated 3 comments.

on:
push:
branches: [ "main", "feat/*" ]
branches: [ "main", "feat/*", "dev/**/*" ]
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This will permit the CodeQL workflow to run on pushes and PRs targeting main, any feature branch, and any dev branch.

include:
# GitHub repo branch targets that will trigger PR validation builds.
- dev/*
- dev/**/*
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This now correctly triggers PR pipeline runs for PRs targeting another dev branch.

{
if (obj != null && obj is TokenCredentialKey tcKey)
{
return _tokenCredentialType == tcKey._tokenCredentialType
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copilot suggested a change here:

#3904 (comment)

Please review carefully, since it changes how the s_tokenCredentialMap dictionary performs key equality!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Area\Azure Connectivity Use this to tag issues that are related to Azure connectivity.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants