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

Fix | MARS header contains errors on .NET Framework 4.8 #910

Merged
merged 3 commits into from
Feb 26, 2021

Conversation

David-Engel
Copy link
Contributor

@David-Engel David-Engel commented Feb 13, 2021

Fixing #85 MARS header contains errors for .NET Framework 4.8.

The underlying issue of why this was failing for .NET Framework 4.8 was that there was conditional logic on the default app context switch, MakeReadAsyncBlocking, setting it to false when the .NET runtime version was > 40702 (inherited from S.D.S netfx code). We don't need that complexity, so the app context code was cleaned up and made mostly common between netfx and netcore code.

This also brings parity to netcore for the existing MakeReadAsyncBlocking switch in netfx. It's already documented. We'll just need to update the doc to reflect that it is now available for .NET Core and .NET Standard, too:
https://docs.microsoft.com/en-us/sql/connect/ado-net/appcontext-switches?view=sql-server-ver15

@David-Engel David-Engel marked this pull request as ready for review February 18, 2021 18:19
@cheenamalhotra cheenamalhotra added the 🆕 Public API Issues/PRs that introduce new APIs to the driver. label Feb 18, 2021
@David-Engel David-Engel changed the title Fix | MARS header contains errors fix Fix | MARS header contains errors on .NET Framework 4.8 Feb 23, 2021
@cheenamalhotra cheenamalhotra added this to the 3.0.0-preview1 milestone Feb 23, 2021
@JRahnama
Copy link
Contributor

LGTM. Couple of minor changes requested. Thank you @David-Engel.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🆕 Public API Issues/PRs that introduce new APIs to the driver.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants