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

Add additional NIOAsyncChannel config #406

Merged
merged 4 commits into from
Jul 4, 2023
Merged

Add additional NIOAsyncChannel config #406

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
c46c98f
Add additional NIOAsyncChannel config
rnro Jul 4, 2023
5d1584d
review comments, re-order parameters
rnro Jul 4, 2023
baa25d5
review comments
rnro Jul 4, 2023
49eb2f9
param reordering, missing docs comments
rnro Jul 4, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions Sources/NIOHTTP2/HTTP2ChannelHandler+InlineStreamMultiplexer.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -255,7 +255,11 @@ extension NIOHTTP2Handler {
/// or ``HTTP2Frame/FramePayload`` if there are none.
/// - initializer: A callback that will be invoked to allow you to configure the
/// `ChannelPipeline` for the newly created channel.
@available(macOS 10.15, iOS 13.0, watchOS 6.0, tvOS 13.0, *)
@_spi(AsyncChannel)
public func createStreamChannel<Inbound, Outbound>(
backpressureStrategy: NIOAsyncSequenceProducerBackPressureStrategies.HighLowWatermark? = nil,
isOutboundHalfClosureEnabled: Bool = false,
inboundType: Inbound.Type,
outboundType: Outbound.Type,
Copy link
Contributor

Choose a reason for hiding this comment

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

can we reorder these so {in,out}boundType come first? Required parameters should come first.

Copy link
Member

Choose a reason for hiding this comment

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

We should add defaults to the types as well like this = Inbound.self. Then the order is the same as in our bootstrap

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't think we want to add defaults here to force people to consider what types are used. This was also mentioned in an earlier PR #403 (comment)

Copy link
Member

Choose a reason for hiding this comment

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

This is not really a default. If you write inboundType: Inbound.Type = Inbound.self it will allow the user to either write out the generic return type which will get inferred or pass parameters. If you don't add this default value then they always have to write out the parameters even though type inference doesn't need it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yep, good point. I've added these in the latest commit and put the parameters back in the order to match NIOAsyncChannel.

initializer: @escaping NIOHTTP2Handler.StreamInitializer
Expand All @@ -264,6 +268,8 @@ extension NIOHTTP2Handler {
initializer(channel).flatMapThrowing { _ in
return try NIOAsyncChannel(
synchronouslyWrapping: channel,
backpressureStrategy: backpressureStrategy,
isOutboundHalfClosureEnabled: isOutboundHalfClosureEnabled,
inboundType: Inbound.self,
outboundType: Outbound.self
)
Expand Down
16 changes: 16 additions & 0 deletions Sources/NIOHTTP2/HTTP2PipelineHelpers.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -296,6 +296,8 @@ extension Channel {
position: ChannelPipeline.Position = .last,
streamInboundType: StreamInbound.Type,
streamOutboundType: StreamOutbound.Type,
inboundStreamBackpressureStrategy: NIOAsyncSequenceProducerBackPressureStrategies.HighLowWatermark? = nil,
isInboundStreamOutboundHalfClosureEnabled: Bool = false,
inboundStreamInitializer: @escaping NIOHTTP2Handler.StreamInitializer
) throws -> EventLoopFuture<NIOHTTP2Handler.AsyncStreamMultiplexer<NIOAsyncChannel<StreamInbound, StreamOutbound>>> {
if self.eventLoop.inEventLoop {
Expand All @@ -308,6 +310,8 @@ extension Channel {
position: position,
streamInboundType: streamInboundType,
streamOutboundType: streamOutboundType,
inboundStreamBackpressureStrategy: inboundStreamBackpressureStrategy,
isInboundStreamOutboundHalfClosureEnabled: isInboundStreamOutboundHalfClosureEnabled,
inboundStreamInitializer: inboundStreamInitializer
)
}
Expand All @@ -321,6 +325,8 @@ extension Channel {
position: position,
streamInboundType: streamInboundType,
streamOutboundType: streamOutboundType,
inboundStreamBackpressureStrategy: inboundStreamBackpressureStrategy,
isInboundStreamOutboundHalfClosureEnabled: isInboundStreamOutboundHalfClosureEnabled,
inboundStreamInitializer: inboundStreamInitializer
)
}
Expand Down Expand Up @@ -529,8 +535,12 @@ extension Channel {
streamDelegate: NIOHTTP2StreamDelegate? = nil,
connectionInboundType: ConnectionInbound.Type,
connectionOutboundType: ConnectionOutbound.Type,
connectionBackpressureStrategy: NIOAsyncSequenceProducerBackPressureStrategies.HighLowWatermark? = nil,
connectionIsOutboundHalfClosureEnabled: Bool = false,
streamInboundType: StreamInbound.Type,
streamOutboundType: StreamOutbound.Type,
inboundStreamBackpressureStrategy: NIOAsyncSequenceProducerBackPressureStrategies.HighLowWatermark? = nil,
isInboundStreamOutboundHalfClosureEnabled: Bool = false,
connectionInitializer: @escaping NIOHTTP2Handler.ConnectionInitializer,
inboundStreamInitializer: @escaping NIOHTTP2Handler.StreamInitializer
) throws -> EventLoopFuture<(
Expand All @@ -544,11 +554,15 @@ extension Channel {
streamDelegate: streamDelegate,
streamInboundType: streamInboundType,
streamOutboundType: streamOutboundType,
inboundStreamBackpressureStrategy: inboundStreamBackpressureStrategy,
isInboundStreamOutboundHalfClosureEnabled: isInboundStreamOutboundHalfClosureEnabled,
inboundStreamInitializer: inboundStreamInitializer
).flatMap { multiplexer in
return connectionInitializer(self).flatMapThrowing { _ in
let connectionAsyncChannel = try NIOAsyncChannel(
synchronouslyWrapping: self,
backpressureStrategy: connectionBackpressureStrategy,
isOutboundHalfClosureEnabled: connectionIsOutboundHalfClosureEnabled,
inboundType: ConnectionInbound.self,
outboundType: ConnectionOutbound.self
)
Expand Down Expand Up @@ -680,6 +694,8 @@ extension ChannelPipeline.SynchronousOperations {
position: ChannelPipeline.Position = .last,
streamInboundType: StreamInbound.Type,
streamOutboundType: StreamOutbound.Type,
inboundStreamBackpressureStrategy: NIOAsyncSequenceProducerBackPressureStrategies.HighLowWatermark? = nil,
isInboundStreamOutboundHalfClosureEnabled: Bool = false,
inboundStreamInitializer: @escaping NIOHTTP2Handler.StreamInitializer
) throws -> NIOHTTP2Handler.AsyncStreamMultiplexer<NIOAsyncChannel<StreamInbound, StreamOutbound>> {
return try self.configureAsyncHTTP2Pipeline(
Expand Down