-
Notifications
You must be signed in to change notification settings - Fork 73
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
Launching Services with existing NWConnection or NWListener objects #156
Conversation
NIO Transport Services is not capable of launching services with existing NWConnection or NWListener objects. Being able to get an existing NWConnection through a connection bootstrap and into a channel is a useful capability for advanced use cases. Modifications: * Added an option to bootstrap with existing NWListener and NWConnection * Completed promise connection earlier within NIOTSChannels when AlreadyConfigured is called * Added test with new NWConnection and NWListener to register Channels Result: Able to create and register a channel using an existing NWListener and NWConnection
Can one of the admins verify this patch? |
5 similar comments
Can one of the admins verify this patch? |
Can one of the admins verify this patch? |
Can one of the admins verify this patch? |
Can one of the admins verify this patch? |
Can one of the admins verify this patch? |
@swift-server-bot add to allowlist |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great, this generally looks really good! Just a few notes left in the diff.
@@ -205,7 +219,11 @@ public final class NIOTSConnectionBootstrap { | |||
initializer(conn) | |||
}.flatMap { | |||
conn.eventLoop.assertInEventLoop() | |||
return conn.register() | |||
if shouldRegister{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if shouldRegister{ | |
if shouldRegister { |
tcpOptions: tcpOptions, | ||
tlsOptions: tlsOptions, | ||
childLoopGroup: childLoopGroup, | ||
childChannelQoS:childChannelQoS, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
childChannelQoS:childChannelQoS, | |
childChannelQoS: childChannelQoS, |
tlsOptions: tlsOptions, | ||
childLoopGroup: childLoopGroup, | ||
childChannelQoS:childChannelQoS, | ||
childTCPOptions:childTCPOptions, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
childTCPOptions:childTCPOptions, | |
childTCPOptions: childTCPOptions, |
childLoopGroup: childLoopGroup, | ||
childChannelQoS:childChannelQoS, | ||
childTCPOptions:childTCPOptions, | ||
childTLSOptions:childTLSOptions |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
childTLSOptions:childTLSOptions | |
childTLSOptions: childTLSOptions |
let buffer = connection.allocator.bufferFor(string: "hello, world!") | ||
let completeFuture = connection.expectRead(buffer) | ||
connection.writeAndFlush(buffer, promise: nil) | ||
// this is the assert that matters to make sure it works writes data |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
// this is the assert that matters to make sure it works writes data | |
// this is the assert that matters to make sure it works & writes data |
@@ -526,7 +555,7 @@ class NIOTSEndToEndTests: XCTestCase { | |||
.connect(to: address) | |||
.wait()) { error in | |||
print(error) | |||
} | |||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
} | |
} |
/// Use a pre-existing `NWConnection` to connect a `Channel`. | ||
/// | ||
/// - parameters: | ||
/// - connection: The NWConnection path to wrap. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/// - connection: The NWConnection path to wrap. | |
/// - connection: The NWConnection to wrap. |
/// Bind the `NIOTSListenerChannel` to an existing `NWListener`. | ||
/// | ||
/// - parameters: | ||
/// - listener: The NWListener path to bind. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/// - listener: The NWListener path to bind. | |
/// - listener: The NWListener to wrap. |
@@ -128,6 +128,28 @@ internal final class NIOTSListenerChannel { | |||
// Must come last, as it requires self to be completely initialized. | |||
self._pipeline = ChannelPipeline(channel: self) | |||
} | |||
|
|||
/// Create a `NIOTSConnectionChannel` with an already-established `NWListener`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/// Create a `NIOTSConnectionChannel` with an already-established `NWListener`. | |
/// Create a `NIOTSListenerChannel` with an already-established `NWListener`. |
NIO Transport Services is not capable of launching services with existing NWConnection or NWListener objects. Being able to get an existing NWConnection through a connection bootstrap and into a channel is a useful capability for advanced use cases.
Modifications:
Result:
Able to create and register a channel using an existing NWListener and NWConnection