Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Fixes ReadAsync() behavior to register Cancellation token action before streaming results #1781

Merged
merged 3 commits into from
Oct 4, 2022

Conversation

cheenamalhotra
Copy link
Member

@cheenamalhotra cheenamalhotra commented Sep 30, 2022

Fixes #1065 with test case added from comment.

cc @jnm2 @roji would appreciate some extra validations!

@cheenamalhotra cheenamalhotra force-pushed the cheena/registerCancellationEarly branch from 6457e7e to 632fa98 Compare September 30, 2022 08:05
@JRahnama
Copy link
Contributor

/azurepipilines run

@cheenamalhotra cheenamalhotra force-pushed the cheena/registerCancellationEarly branch from e607d46 to 49440fa Compare September 30, 2022 21:00
@cheenamalhotra cheenamalhotra force-pushed the cheena/registerCancellationEarly branch from 51f50d9 to e79cfa6 Compare October 1, 2022 00:18
@codecov
Copy link

codecov bot commented Oct 1, 2022

Codecov Report

Base: 71.47% // Head: 71.43% // Decreases project coverage by -0.04% ⚠️

Coverage data is based on head (e79cfa6) compared to base (f62ac3b).
Patch coverage: 100.00% of modified lines in pull request are covered.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1781      +/-   ##
==========================================
- Coverage   71.47%   71.43%   -0.05%     
==========================================
  Files         293      293              
  Lines       61381    61381              
==========================================
- Hits        43872    43846      -26     
- Misses      17509    17535      +26     
Flag Coverage Δ
addons 92.38% <ø> (ø)
netcore 74.99% <100.00%> (-0.05%) ⬇️
netfx 69.29% <100.00%> (-0.03%) ⬇️

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

Impacted Files Coverage Δ
...core/src/Microsoft/Data/SqlClient/SqlDataReader.cs 72.84% <100.00%> (+0.04%) ⬆️
...etfx/src/Microsoft/Data/SqlClient/SqlDataReader.cs 68.48% <100.00%> (+0.08%) ⬆️
...ata/SqlClient/SqlConnectionTimeoutErrorInternal.cs 41.96% <0.00%> (-9.83%) ⬇️
...Microsoft/Data/ProviderBase/DbConnectionFactory.cs 17.04% <0.00%> (-3.41%) ⬇️
...soft/Data/SqlClient/TdsParserStateObjectManaged.cs 84.02% <0.00%> (-1.39%) ⬇️
...c/Microsoft/Data/SqlClient/SqlConnectionFactory.cs 68.85% <0.00%> (-0.82%) ⬇️
...rc/Microsoft/Data/ProviderBase/DbConnectionPool.cs 85.19% <0.00%> (-0.67%) ⬇️
...rc/Microsoft/Data/ProviderBase/DbConnectionPool.cs 85.63% <0.00%> (-0.59%) ⬇️
...crosoft/Data/SqlClient/SqlInternalConnectionTds.cs 72.88% <0.00%> (-0.40%) ⬇️
...ient/netfx/src/Microsoft/Data/SqlClient/SqlUtil.cs 58.29% <0.00%> (-0.25%) ⬇️
... and 5 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

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

@David-Engel David-Engel added this to the 5.1.0-preview1 milestone Oct 4, 2022
@lcheunglci
Copy link
Contributor

@cheenamalhotra Please backport this to the 5.0.1 as well. Thanks!

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Triggering a cancellation token may not cancel a running query based on timing
6 participants