Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Fix | Fixing issue for SQL named Instances in Managed SNI using named pipes. #2142

Merged
merged 23 commits into from
Oct 31, 2023
Merged

Fix | Fixing issue for SQL named Instances in Managed SNI using named pipes. #2142

merged 23 commits into from
Oct 31, 2023

Conversation

arellegue
Copy link
Contributor

@arellegue arellegue commented Aug 31, 2023 •
edited by JRahnama
Loading

fixes #2113

@arellegue arellegue linked an issue Aug 31, 2023 that may be closed by this pull request
Named pipes (np:) don't support named instances in Managed SNI #2113
Closed
@codecov
Copy link

codecov bot commented Sep 1, 2023 •
edited
Loading

Codecov Report

Attention: 8 lines in your changes are missing coverage. Please review.

Files Coverage Δ
...re/src/Microsoft/Data/SqlClient/SNI/SNINpHandle.cs 7.81% <0.00%> (ø)
...tcore/src/Microsoft/Data/SqlClient/SNI/SNIProxy.cs 48.45% <30.00%> (-1.01%) ⬇️

... and 32 files with indirect coverage changes

📢 Thoughts on this report? Let us know!.

@JRahnama JRahnama changed the title Fix | Implement support for NamedPiped Instance in Managed SNI. Fix | for NamedPiped Instance in Managed SNI. Sep 1, 2023
@JRahnama JRahnama changed the title Fix | for NamedPiped Instance in Managed SNI. Fix | Fixing NamedPiped issue for SQL Instances in Managed SNI. Sep 1, 2023
@JRahnama JRahnama changed the title Fix | Fixing NamedPiped issue for SQL Instances in Managed SNI. Fix | Fixing issue for SQL named Instances in Managed SNI using named pipes. Sep 1, 2023
@DavoudEshtehari DavoudEshtehari added this to the 5.2.0-preview4 milestone Sep 22, 2023
@DavoudEshtehari DavoudEshtehari added the Ⓜ️ Managed SNI Use this label if the issue/PR relates to issues in Managed SNI label Sep 22, 2023
@DavoudEshtehari DavoudEshtehari marked this pull request as ready for review September 22, 2023 22:52
@arellegue
Enhance InstanceNameTest to get NormalizedPipePath using either m_nor…
4dbd383 
…malizedPipePath or _normalizedPipePath whichever worked.

Disable platform checking for non-windows but comment that it is for WSL debugging.
JRahnama
JRahnama reviewed Sep 25, 2023
View reviewed changes
src/NuGet.config Outdated Show resolved Hide resolved
@JRahnama
Copy link
Member

@arellegue are the failures addressed on the pipelines?

@arellegue
Copy link
Contributor Author

@arellegue are the failures addressed on the pipelines?

The CI-SqlClient pipeline is now passing for this PR. The Named Instance Test Job is disabled for now in CI-SqlClient pipeline. It will be enabled once this PR is approved.

@DavoudEshtehari DavoudEshtehari merged commit d5df671 into dotnet:main Oct 31, 2023
121 of 133 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@David-Engel David-Engel David-Engel approved these changes

@DavoudEshtehari DavoudEshtehari DavoudEshtehari approved these changes

@JRahnama JRahnama Awaiting requested review from JRahnama

Assignees
No one assigned
Labels
Ⓜ️ Managed SNI Use this label if the issue/PR relates to issues in Managed SNI
Projects
None yet
Milestone
5.2.0-preview4
Development

Successfully merging this pull request may close these issues.

Named pipes (np:) don't support named instances in Managed SNI
4 participants
@arellegue @JRahnama @David-Engel @DavoudEshtehari