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.

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

Merge to shared - TdsParserSafeHandles #1604

Merged

Conversation

lcheunglci
Copy link
Contributor

Relates to #1261. Merged netfx into netcore for TdsParserSafeHandles, and moving it to shared src and update the project references.

timeout = Timeout.Infinite; // -1 == native SNIOPEN_TIMEOUT_VALUE / INFINITE
}

#if NETFRAMEWORK
int transparentNetworkResolutionStateNo = (int)transparentNetworkResolutionState;
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't know what the plans are for transparent network resolution. It may be a feature that should come to netcore eventually. It might be an idea to have a new compiler constant for it, @David-Engel is probably the person who would know.

Copy link
Contributor

Choose a reason for hiding this comment

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

The current sentiment around TNIR is that we can bring connection string and SqlConnectionStringBuilder parity to netcore but we should not actually implement the TNIR logic. Apparently it caused more support issues than it solved for Azure. Customers that really need the driver to initiate parallel connection attempts against each IP DNS resolves to should use MSF.

@lcheunglci
Copy link
Contributor Author

Thanks for the review @Wraith2 and I made the revisions. I noticed that there were some compiler errors on Linux because this file was meant for windows only and I placed it in the common ItemGroup. I also noticed that some existing files have the .windows suffix, not sure if I should add the windows suffix to this to keep it consistent.

@Wraith2
Copy link
Contributor

Wraith2 commented May 5, 2022

If it's windows specific then please add the .windows.cs suffix.

@codecov
Copy link

codecov bot commented Jul 12, 2022

Codecov Report

Merging #1604 (8669b08) into main (4e3aa5e) will decrease coverage by 0.16%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main    #1604      +/-   ##
==========================================
- Coverage   71.54%   71.38%   -0.17%     
==========================================
  Files         291      290       -1     
  Lines       61241    61147      -94     
==========================================
- Hits        43817    43651     -166     
- Misses      17424    17496      +72     
Flag Coverage Δ
addons 92.38% <ø> (ø)
netcore 74.90% <100.00%> (-0.19%) ⬇️
netfx 69.24% <100.00%> (-0.11%) ⬇️

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

Impacted Files Coverage Δ
...nt/netfx/src/Microsoft/Data/SqlClient/TdsParser.cs 71.07% <100.00%> (-0.04%) ⬇️
...oft/Data/SqlClient/TdsParserSafeHandles.Windows.cs 93.20% <100.00%> (ø)
...src/Microsoft/Data/SqlClient/SqlMetadataFactory.cs 81.66% <0.00%> (-10.84%) ⬇️
...re/src/Microsoft/Data/SqlClient/SNI/SNINpHandle.cs 68.85% <0.00%> (-9.84%) ⬇️
...ata/SqlClient/SqlConnectionTimeoutErrorInternal.cs 41.96% <0.00%> (-9.83%) ⬇️
...ActiveDirectoryAuthenticationTimeoutRetryHelper.cs 56.81% <0.00%> (-6.82%) ⬇️
.../src/Microsoft/Data/SqlClient/SNI/SNIMarsHandle.cs 87.89% <0.00%> (-4.49%) ⬇️
...nt/src/Microsoft/Data/ProviderBase/TimeoutTimer.cs 71.42% <0.00%> (-3.58%) ⬇️
...Microsoft/Data/ProviderBase/DbConnectionFactory.cs 17.04% <0.00%> (-3.41%) ⬇️
...crosoft/Data/ProviderBase/DbReferenceCollection.cs 92.63% <0.00%> (-3.16%) ⬇️
... and 13 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4e3aa5e...8669b08. Read the comment docs.

@JRahnama JRahnama added the ➕ Code Health Issues/PRs that are targeted to source code quality improvements. label Aug 16, 2022
@JRahnama JRahnama added this to the 5.1.0-preview1 milestone Aug 16, 2022
@DavoudEshtehari DavoudEshtehari merged commit ac7c1a4 into dotnet:main Oct 5, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
➕ Code Health Issues/PRs that are targeted to source code quality improvements.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants