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

Add async TCP echo example #2463

Merged
merged 5 commits into from
Jul 31, 2023
Merged

Conversation

FranzBusch
Copy link
Member

Motivation

We introduced new async APIs to our bootstraps that make it easy to use NIO from Swift Concurrency. We should provide examples showing how to use those new APIs. In the future, we want to make those examples the primary ones and remove the non-async based ones.

Modification

This PR introduces two new targets: NIOTCPEchoServer and NIOTCPEchoClient. Both targets are executable and provide either the server or client piece of the example.

Result

New and shiny async example

@FranzBusch FranzBusch requested review from Lukasa and glbrntt July 7, 2023 09:49
@FranzBusch FranzBusch force-pushed the fb-async-tcp-echo branch 4 times, most recently from 438e2d1 to e82a6c5 Compare July 11, 2023 16:01
Copy link
Member

@weissi weissi left a comment

Choose a reason for hiding this comment

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

Just leaking an EventLoopGroup in a function is illegal (will get deinited without shutdown which is a lifecycle violation)

@FranzBusch FranzBusch requested review from weissi and glbrntt July 27, 2023 12:27
@FranzBusch
Copy link
Member Author

@weissi I updated this PR to use your newly landed singletons.

@FranzBusch FranzBusch requested a review from weissi July 27, 2023 14:13
) { channel in
channel.eventLoop.makeCompletedFuture {
// We are using two simple handlers here to frame our messages with "\n"
try channel.pipeline.syncOperations.addHandler(ByteToMessageHandler(NewlineDelimiterCoder()))
Copy link
Member

Choose a reason for hiding this comment

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

all these trys should be try!s, they can't fail unless programmer error, right?

Copy link
Member Author

Choose a reason for hiding this comment

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

They could throw if the channel is closed while we add the handler. I don't wanna show adopters a try! when it could fail in rare cases and I don't think the try is too bad.

@weissi weissi self-requested a review July 27, 2023 14:20
@FranzBusch FranzBusch requested a review from glbrntt July 27, 2023 17:56
Copy link
Member

@weissi weissi left a comment

Choose a reason for hiding this comment

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

lgtm

# Motivation
We introduced new async APIs to our bootstraps that make it easy to use NIO from Swift Concurrency. We should provide examples showing how to use those new APIs. In the future, we want to make those examples the primary ones and remove the non-async based ones.

# Modification
This PR introduces two new targets: `NIOTCPEchoServer` and `NIOTCPEchoClient`. Both targets are executable and provide either the server or client piece of the example.

# Result
New and shiny async example
@FranzBusch FranzBusch added the semver/none No version bump required. label Jul 31, 2023
@FranzBusch FranzBusch enabled auto-merge (squash) July 31, 2023 08:32
@FranzBusch FranzBusch merged commit b25bc58 into apple:main Jul 31, 2023
@FranzBusch FranzBusch deleted the fb-async-tcp-echo branch July 31, 2023 08:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
semver/none No version bump required.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants