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 th-TH culture info issue on Managed SNI #2066

Merged
merged 14 commits into from
Jun 28, 2023
Merged

Fix th-TH culture info issue on Managed SNI #2066

merged 14 commits into from
Jun 28, 2023

Conversation

arellegue
Copy link
Contributor

@arellegue arellegue commented Jun 16, 2023 •
edited by DavoudEshtehari
Loading

fixes #1579

@codecov
Copy link

codecov bot commented Jun 16, 2023 •
edited
Loading

Codecov Report

Patch coverage: 50.00% and project coverage change: -0.58 ⚠️

Comparison is base (2b31810) 70.80% compared to head (0495369) 70.22%.

❗ Current head 0495369 differs from pull request most recent head 7e3a257. Consider uploading reports for the commit 7e3a257 to get more accurate results

Additional details and impacted files 
@@            Coverage Diff             @@
##             main    #2066      +/-   ##
==========================================
- Coverage   70.80%   70.22%   -0.58%     
==========================================
  Files         305      305              
  Lines       61819    61819              
==========================================
- Hits        43770    43413     -357     
- Misses      18049    18406     +357
Flag Coverage Δ
addons 92.88% <ø> (ø)
netcore 72.06% <50.00%> (-1.46%) ⬇️
netfx 69.25% <ø> (-0.19%) ⬇️

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

Impacted Files Coverage Δ
...tcore/src/Microsoft/Data/SqlClient/SNI/SNIProxy.cs 49.46% <50.00%> (ø)

... and 28 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@DavoudEshtehari DavoudEshtehari changed the title Issue 1579 th-TH culture info causes network connection error Fix th-TH culture info issue on Managed SNI Jun 17, 2023
@DavoudEshtehari DavoudEshtehari added the Area\Managed SNI Issues that are targeted to the Managed SNI codebase. label Jun 17, 2023
@DavoudEshtehari DavoudEshtehari added this to the 5.2.0-preview3 milestone Jun 17, 2023
@arellegue arellegue requested a review from David-Engel June 19, 2023 23:29
@arellegue
Move ConnectionTestWithCutltureTH back to SqlConnectionBasicTests.
650cc6c 
Replaced [ConditionalFact] with just [Fact]
Added catch { throw; } back.
@DavoudEshtehari DavoudEshtehari merged commit 20ab3b1 into dotnet:main Jun 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@JRahnama JRahnama JRahnama approved these changes

@Wraith2 Wraith2 Wraith2 left review comments

@roji roji roji left review comments

@DavoudEshtehari DavoudEshtehari DavoudEshtehari approved these changes

@Kaur-Parminder Kaur-Parminder Awaiting requested review from Kaur-Parminder

@David-Engel David-Engel Awaiting requested review from David-Engel

Assignees
No one assigned
Labels
Area\Managed SNI Issues that are targeted to the Managed SNI codebase.
Projects
None yet
Milestone
5.2.0-preview3
Development

Successfully merging this pull request may close these issues.

Getting ExtendedSocketException in "th-TH" culture SqlConnection.Open method
6 participants
@arellegue @roji @Wraith2 @David-Engel @JRahnama @DavoudEshtehari