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

ChannelPipeline H2 inline multiplexer conf funcs #381

Conversation

rnro
Copy link
Contributor

@rnro rnro commented Mar 30, 2023

Motivation:

To have parity with the legacy separate HTTP2ChannelHandler-StreamMultiplexer HTTP2 stream multiplexing model we provide channel pipelineconfiguration functions using the new inline multiplexer.

Modifications:

  • New versions of configureHTTP2Pipeline and configureCommonHTTPServerPipeline which set up the pipeline using inline stream multiplexers
  • Tests targeting the new functions ported from ConfiguringPipelineWithFramePayloadStreamsTests.swift

Result:

New pipeline configuration conveniences for inline HTTP2 multiplexing are available.

Motivation:

To have parity with the legacy separate `HTTP2ChannelHandler`-`StreamMultiplexer` HTTP2 stream multiplexing model we provide channel pipelineconfiguration functions using the new inline multiplexer.

Modifications:

* New versions of `configureHTTP2Pipeline` and
  `configureCommonHTTPServerPipeline` which set up the pipeline using
inline stream multiplexers
* Tests targeting the new functions ported from
  `ConfiguringPipelineWithFramePayloadStreamsTests.swift`

Result:

New pipeline configuration conveniences for inline HTTP2 multiplexing
are available.
@rnro rnro force-pushed the ChannelPipeline_HTTP2_pipeline_configuration_functions_for_inline_multiplexer branch from 78863f8 to ea28449 Compare March 30, 2023 15:02
@Lukasa Lukasa added the semver/minor Adds new public API. label Mar 30, 2023
return self.configureHTTP2SecureUpgrade(h2ChannelConfigurator: h2ChannelConfigurator,
http1ChannelConfigurator: http1ChannelConfigurator)
}
}
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 find a way to deduplicate this with the older code?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes! Somewhat. There is some duplicated code but I don't think we'd be able to save much by abstracting it away further.

@rnro rnro requested a review from Lukasa March 30, 2023 16:19
@rnro rnro merged commit a32335d into apple:main Mar 30, 2023
@rnro rnro deleted the ChannelPipeline_HTTP2_pipeline_configuration_functions_for_inline_multiplexer branch June 12, 2023 08:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
semver/minor Adds new public API.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants