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

implement ConnectAsync overloads with CancellationToken on Socket and TcpClient #40750

Merged
merged 8 commits into from
Aug 16, 2020
Merged

implement ConnectAsync overloads with CancellationToken on Socket and TcpClient #40750

merged 8 commits into from
Aug 16, 2020

Conversation

geoffkizer
Copy link
Contributor

Contributes to #33418

@stephentoub @dotnet/ncl

@Dotnet-GitSync-Bot
Copy link
Collaborator

Note regarding the new-api-needs-documentation label:

This serves as a reminder for when your PR is modifying a ref *.cs file and adding/modifying public APIs, to please make sure the API implementation in the src *.cs file is documented with triple slash comments, so the PR reviewers can sign off that change.

@ghost
Copy link

ghost commented Aug 13, 2020

Tagging subscribers to this area: @dotnet/ncl
See info in area-owners.md if you want to be subscribed.

@geoffkizer geoffkizer self-assigned this Aug 13, 2020
@geoffkizer geoffkizer added this to the 5.0.0 milestone Aug 13, 2020
@geoffkizer
Copy link
Contributor Author

/azp run runtime-libraries outerloop

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

stephentoub
stephentoub reviewed Aug 13, 2020
View reviewed changes
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.

Can we use this now in the connection factory stuff?

src/libraries/System.Net.Sockets/src/System/Net/Sockets/Socket.Tasks.cs Outdated Show resolved Hide resolved
src/libraries/System.Net.Sockets/src/System/Net/Sockets/Socket.Tasks.cs Show resolved Hide resolved
src/libraries/System.Net.Sockets/src/System/Net/Sockets/Socket.Tasks.cs Show resolved Hide resolved
@antonfirsov
Copy link
Member

Can we use this now in the connection factory stuff?

Good question which PR gets merged first. If mine, we can do it in a follow-up PR.

@geoffkizer
Copy link
Contributor Author

/azp run runtime-libraries outerloop

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@jaredpar jaredpar mentioned this pull request Aug 13, 2020
Closed
@geoffkizer geoffkizer requested a review from a team August 13, 2020 22:17
@geoffkizer
Copy link
Contributor Author

/azp run runtime-libraries outerloop

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

stephentoub
stephentoub approved these changes Aug 14, 2020
View reviewed changes
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.

LGTM

@geoffkizer
Copy link
Contributor Author

I also modified SocketConnectionFactory to use the new ConnectAsync API.

@geoffkizer
Copy link
Contributor Author

/azp run runtime-libraries outerloop

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@geoffkizer
Copy link
Contributor Author

/azp run runtime-libraries outerloop

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@geoffkizer
Copy link
Contributor Author

/azp run runtime-libraries outerloop

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@geoffkizer geoffkizer merged commit 1341c35 into dotnet:master Aug 16, 2020
@geoffkizer geoffkizer mentioned this pull request Aug 28, 2020
Closed
@roji roji mentioned this pull request Sep 20, 2020
Closed
@MartyIX
Copy link
Contributor

MartyIX commented Oct 6, 2020

@geoffkizer Is public Task ConnectAsync(IPEndPoint remoteEP) missing on purpose in TcpClient ?

@geoffkizer
Copy link
Contributor Author

I think it just never got added. Feel free to file an issue on it.

@MartyIX MartyIX mentioned this pull request Oct 6, 2020
Closed
@geoffkizer geoffkizer deleted the connectasynccancellation branch November 7, 2020 23:40
@ghost ghost locked as resolved and limited conversation to collaborators Dec 8, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@antonfirsov antonfirsov antonfirsov left review comments

@stephentoub stephentoub stephentoub approved these changes

Assignees

@geoffkizer geoffkizer

Labels
area-System.Net.Sockets new-api-needs-documentation
Projects
None yet
Milestone
5.0.0
Development

Successfully merging this pull request may close these issues.
5 participants
@geoffkizer @Dotnet-GitSync-Bot @antonfirsov @MartyIX @stephentoub