Skip to content

Conversation

@mdaigle
Copy link
Contributor

@mdaigle mdaigle commented Oct 20, 2025

Description

Backports #3625

@mdaigle mdaigle marked this pull request as ready for review October 20, 2025 22:11
Copilot AI review requested due to automatic review settings October 20, 2025 22:11
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 PR backports a fix for Basic Availability Group (BAG) failover behavior by introducing an AppContext switch that allows applications to ignore server-provided failover partners. This enables applications to maintain explicit control over failover configuration (e.g., using custom ports) instead of automatically accepting server-suggested partners.

Key changes include:

  • Added a new IgnoreServerProvidedFailoverPartner AppContext switch with documentation
  • Refactored ServerProvidedFailoverPartner from a private field with getter to an auto-property
  • Updated failover logic to respect the new switch when determining actual failover partners

Reviewed Changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/LocalAppContextSwitches.cs Adds the new IgnoreServerProvidedFailoverPartner AppContext switch with property accessor and fixes a typo in existing documentation
src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/SqlInternalConnectionTds.cs Refactors failover partner storage, implements switch logic to conditionally use server-provided or connection-string failover partners
src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/SqlCommand.cs Updates property reference to use the new naming convention
src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SqlInternalConnectionTds.cs Mirrors the netfx implementation for .NET Core, applying the same refactoring and switch logic
src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SqlCommand.cs Updates property reference to use the new naming convention

@mdaigle mdaigle requested a review from a team October 20, 2025 22:12
@mdaigle mdaigle added this to the 6.0.4 milestone Oct 20, 2025
@codecov
Copy link

codecov bot commented Oct 20, 2025

Codecov Report

❌ Patch coverage is 72.00000% with 7 lines in your changes missing coverage. Please review.
✅ Project coverage is 75.69%. Comparing base (1542afd) to head (cccd71c).
⚠️ Report is 1 commits behind head on release/6.0.

Files with missing lines Patch % Lines
...crosoft/Data/SqlClient/SqlInternalConnectionTds.cs 68.42% 6 Missing ⚠️
...icrosoft/Data/SqlClient/LocalAppContextSwitches.cs 80.00% 1 Missing ⚠️
Additional details and impacted files
@@               Coverage Diff               @@
##           release/6.0    #3703      +/-   ##
===============================================
+ Coverage        75.51%   75.69%   +0.17%     
===============================================
  Files              244      244              
  Lines            40212    40221       +9     
===============================================
+ Hits             30368    30445      +77     
+ Misses            9844     9776      -68     
Flag Coverage Δ
addons 92.58% <ø> (ø)
netcore 75.56% <72.00%> (+0.17%) ⬆️

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.

benrr101
benrr101 previously approved these changes Oct 21, 2025
Copy link
Contributor

@benrr101 benrr101 left a comment

Choose a reason for hiding this comment

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

I don't really feel strongly enough about "" vs string.Empty to reject. But I'd happily re-approve if you did want to take that change.

@paulmedynski paulmedynski self-assigned this Oct 24, 2025
@mdaigle mdaigle merged commit 84d914c into release/6.0 Oct 28, 2025
240 checks passed
@mdaigle mdaigle deleted the dev/mdaigle/6.0-bag-failover-fix branch October 28, 2025 20:52
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.

4 participants