Generic helpers for HTTP/2 async pipelines #401
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Motivation: In preparation for exposing APIs which surface HTTP/2 connections and streams using structured concurrency this PR introduces a store for inbound HTTP/2 streams. Modifications: Define types and methods for storing generic types which wrap `Channels` corresponding to inbound HTTP/2 streams in an async sequence. Result: The new types are not yet exposed, this work introduces part of the framework for future functionality.
Motivation: This PR is the first step in exposing APIs which surface HTTP/2 connections and streams using structured concurrency. This PR exposes the most abstract spelling of this concept, assuming no particular types/forms for the types involved in the stream channel types. Modifications: * Store a generic `streamInitializer` on the common inbound initializer which is used to initialize inbound streams before yielding them to the continuation of streams. * Expose pipeline configuration functions which assume HTTP/2 but nothing else about sream channel types. Result: Adopters of the new SPI should be able to create outbound and deal with inbound HTTP/2 stream channels using async streams. Outside of SPI there should be no changes.
* Finish stream channel sequence continuation * Provide internal functions for creating streams without configuring them. Configuring leads to activation so it can be helpful to allow that step to be performed manually once any provided initialization closures have beenn executed.
FranzBusch
requested changes
Jun 26, 2023
Sources/NIOHTTP2/HTTP2ChannelHandler+InlineStreamMultiplexer.swift
Outdated
Show resolved
Hide resolved
Sources/NIOHTTP2/HTTP2ChannelHandler+InlineStreamMultiplexer.swift
Outdated
Show resolved
Hide resolved
Sources/NIOHTTP2/HTTP2ChannelHandler+InlineStreamMultiplexer.swift
Outdated
Show resolved
Hide resolved
Sources/NIOHTTP2/HTTP2ChannelHandler+InlineStreamMultiplexer.swift
Outdated
Show resolved
Hide resolved
Tests/NIOHTTP2Tests/ConfiguringPipelineAsyncMultiplexerTests.swift
Outdated
Show resolved
Hide resolved
Tests/NIOHTTP2Tests/ConfiguringPipelineAsyncMultiplexerTests.swift
Outdated
Show resolved
Hide resolved
glbrntt
requested changes
Jun 26, 2023
Sources/NIOHTTP2/HTTP2ChannelHandler+InlineStreamMultiplexer.swift
Outdated
Show resolved
Hide resolved
Sources/NIOHTTP2/HTTP2ChannelHandler+InlineStreamMultiplexer.swift
Outdated
Show resolved
Hide resolved
Sources/NIOHTTP2/HTTP2ChannelHandler+InlineStreamMultiplexer.swift
Outdated
Show resolved
Hide resolved
Sources/NIOHTTP2/HTTP2ChannelHandler+InlineStreamMultiplexer.swift
Outdated
Show resolved
Hide resolved
glbrntt
reviewed
Jun 27, 2023
Sources/NIOHTTP2/HTTP2ChannelHandler+InlineStreamMultiplexer.swift
Outdated
Show resolved
Hide resolved
rnro
force-pushed
the
async_connection_channel
branch
from
6fea265
to
e9c7f8e
Compare
rnro
force-pushed
the
async_connection_channel
branch
from
737f00c
to
3a65355
Compare
glbrntt
reviewed
Jun 28, 2023
FranzBusch
reviewed
Jun 29, 2023
Sources/NIOHTTP2/HTTP2ChannelHandler+InlineStreamMultiplexer.swift
Outdated
Show resolved
Hide resolved
Comment on lines
101
to
102
| let serverRecorder = InboundFramePayloadRecorder() | ||
| serverRecorderContinuation.yield(serverRecorder) |
There was a problem hiding this comment.
I don't think this is safe. The InboundFramePayloadRecorder is probably not Sendable. What I meant with my previous comment was actually passing the continuation into this recorder so that the recorder is yielding whatever it records to it. (We might want to create a complete new type for this instead of botching the InboundFramePayloadRecorder)
There was a problem hiding this comment.
Yeah I realized afterwards that this is probably not what you meant however I think it works as a convenient way to get the recorders out of the initializers and indicate when they're 'done'. I have made InboundFramePayloadRecorder Sendable to make it safe.
glbrntt
reviewed
Jun 29, 2023
Tests/NIOHTTP2Tests/ConfiguringPipelineAsyncMultiplexerTests.swift
Outdated
Show resolved
Hide resolved
Tests/NIOHTTP2Tests/ConfiguringPipelineAsyncMultiplexerTests.swift
Outdated
Show resolved
Hide resolved
glbrntt
reviewed
Jun 29, 2023
Tests/NIOHTTP2Tests/ConfiguringPipelineAsyncMultiplexerTests.swift
Outdated
Show resolved
Hide resolved
Tests/NIOHTTP2Tests/ConfiguringPipelineAsyncMultiplexerTests.swift
Outdated
Show resolved
Hide resolved
Tests/NIOHTTP2Tests/ConfiguringPipelineAsyncMultiplexerTests.swift
Outdated
Show resolved
Hide resolved
Tests/NIOHTTP2Tests/ConfiguringPipelineAsyncMultiplexerTests.swift
Outdated
Show resolved
Hide resolved
rnro
force-pushed
the
async_connection_channel
branch
from
c31c408
to
34fe176
Compare
rnro
force-pushed
the
async_connection_channel
branch
from
34fe176
to
43676e8
Compare
glbrntt
reviewed
Jun 30, 2023
Tests/NIOHTTP2Tests/ConfiguringPipelineAsyncMultiplexerTests.swift
Outdated
Show resolved
Hide resolved
glbrntt
reviewed
Jun 30, 2023
Tests/NIOHTTP2Tests/ConfiguringPipelineAsyncMultiplexerTests.swift
Outdated
Show resolved
Hide resolved
glbrntt
approved these changes
Jun 30, 2023
Lukasa
added
the
patch-version-bump-only
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Expose (as spi) generic helpers for HTTP/2 async pipelines
Motivation:
This PR is the first step in exposing APIs which surface HTTP/2 connections/streams using structured concurrency. This PR exposes the most abstract spelling of this concept, assuming no particular types/forms for the types involved in the stream channel types.
Modifications:
Channelscorresponding to inbound HTTP/2 streams in an async sequence.streamInitializeron the common inbound initializer which is used to initialize inbound streams before yielding them to the continuation of streams.Result:
Adopters of the new SPI should be able to create outbound and deal with inbound HTTP/2 stream channels using async streams.
Outside of SPI there should be no changes.