Skip to content
This repository has been archived by the owner on Jan 23, 2023. It is now read-only.
/ corefx Public archive

Remove cancellation token from new System.Data CloseAsync() #39070

Merged
merged 1 commit into from
Jul 2, 2019

Conversation

roji
Copy link
Member

@roji roji commented Jul 1, 2019

Affects DbDataReader and DbConnection, since these APIs are very likely to be used for cleanup only, in which case a cancellation token is an anti-pattern (similar to why DisposeAsync doesn't accept one).

/cc @stephentoub @terrajobst @divega @ajcvickers

See discussion here: dotnet/standard#1283 (review) (I propose to keep conversation in that issue)

Fixes #39060

@divega
Copy link

divega commented Jul 1, 2019

@roji this would fix #39069.

Affects DbDataReader and DbConnection, since these APIs are very likely
to be used for cleanup only, in which case a cancellation token is
an anti-pattern (similar to why DisposeAsync doesn't accept one).

See discussion here:
dotnet/standard#1283 (review)

Fixes #39069
@roji roji force-pushed the SystemDataRemoveCloseCancellation branch from edc66c3 to 2f889e9 Compare July 1, 2019 11:28
@roji
Copy link
Member Author

roji commented Jul 1, 2019

Thanks, my head got turned around with all the issues around this - amended the commit.

@stephentoub
Copy link
Member

cc: @terrajobst, @danmosemsft

This is a post-Preview7 breaking API change. Presumably it needs more scrutiny?

Copy link
Member

@stephentoub stephentoub left a comment

Choose a reason for hiding this comment

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

The change itself looks good, but it needs to be approved higher-up before it's merged, as we're now into no-API-breaking-change territory.

@stephentoub stephentoub added the * NO MERGE * The PR is not ready for merge yet (see discussion for detailed reasons) label Jul 1, 2019
@stephentoub stephentoub added this to the 3.0 milestone Jul 1, 2019
@wtgodbe
Copy link
Member

wtgodbe commented Jul 2, 2019

Approved for preview7

@wtgodbe wtgodbe merged commit 539ed31 into dotnet:master Jul 2, 2019
@wtgodbe wtgodbe removed the * NO MERGE * The PR is not ready for merge yet (see discussion for detailed reasons) label Jul 2, 2019
wtgodbe pushed a commit to wtgodbe/corefx that referenced this pull request Jul 2, 2019
…9070)

Affects DbDataReader and DbConnection, since these APIs are very likely
to be used for cleanup only, in which case a cancellation token is
an anti-pattern (similar to why DisposeAsync doesn't accept one).

See discussion here:
dotnet/standard#1283 (review)

Fixes #39069
roji added a commit to roji/standard that referenced this pull request Jul 2, 2019
wtgodbe pushed a commit to dotnet/standard that referenced this pull request Jul 2, 2019
@roji roji deleted the SystemDataRemoveCloseCancellation branch July 2, 2019 19:40
wtgodbe added a commit that referenced this pull request Jul 2, 2019
…39122)

Affects DbDataReader and DbConnection, since these APIs are very likely
to be used for cleanup only, in which case a cancellation token is
an anti-pattern (similar to why DisposeAsync doesn't accept one).

See discussion here:
dotnet/standard#1283 (review)

Fixes #39069
picenka21 pushed a commit to picenka21/runtime that referenced this pull request Feb 18, 2022
…orefx#39070)

Affects DbDataReader and DbConnection, since these APIs are very likely
to be used for cleanup only, in which case a cancellation token is
an anti-pattern (similar to why DisposeAsync doesn't accept one).

See discussion here:
dotnet/standard#1283 (review)

Fixes dotnet/corefx#39069

Commit migrated from dotnet/corefx@539ed31
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Error while checking for terminated children
4 participants