HTTP/3 connection params #151
Conversation
|
Should we keep the client/server builders? Also, any suggestions on the name of the parameter builder? |
I would keep the client/server builders because there will possibly be parameters which only belong to server or client.
Maybe just |
h3/src/params.rs
Outdated
| pub const DEFAULT_MAX_FIELD_SECTION_SIZE: u64 = (1 << 62) - 1; | ||
|
|
||
| /// Set whether WebTransport is supported | ||
| pub fn enable_webtransport(mut self, val: bool) -> Self { |
There was a problem hiding this comment.
It might be better to put the WebTransport stuff behind a Feature Flag. What do you think?
There was a problem hiding this comment.
Yeah, I thought about that too. But what if it's just for params and settings?
There was a problem hiding this comment.
I would put WebTransport completely behind a feature flag or maybe even in a separate crate. Iirc there was some discussion about this in #71.
There was a problem hiding this comment.
I understand the benefits of putting stuff behind a feature gate sometimes (smaller binary, faster compilation, etc). I totally agree we should feature-gate or separate major implementations of WT.
But configs and settings are lightweight and hard to separate. The benefit of putting those behind a feature gate seems limited. I'm 100% sure about this though. What do you think? @seanmonstar
There was a problem hiding this comment.
I thought about it more from a User perspective. Because it may lead to confusion when you are able to set a config which will do nothing without the feature.
There was a problem hiding this comment.
Yeah, it makes sense. And adding that may be too soon..
There was a problem hiding this comment.
I have not had the time to read the WebTransport spec, so not sure what more is needed, but: does adding this at least allow for WebTransport to be proxied? If it's needed for a proxy to simply pass the data frames along, then seems fair to include it as part of the crate, perhaps?
There was a problem hiding this comment.
Yeah, I think so. The setting would be required to be sent by both sides when establishing connection to create a WT session.
|
The API still feels a bit weird. There should probably be one builder instead of two: h3::config::Config {}
h3::client::Builder { config }
h3::server::Builder { config }So, it should either be something that looks like // somehow dedupped
h3::config::ClientConfig {}
h3::config::ServerConfig {}
// no more Builder
h3::client::new(conn, client_config)
h3::server::new(conn, server_config)or // somehow dedupped
h3::Endpoint::client().build(conn)
h3::Endpoint::server().build(conn) |
|
I like this option better. // somehow dedupped
h3::config::ClientConfig {}
h3::config::ServerConfig {}
// no more Builder
h3::client::new(conn, client_config)
h3::server::new(conn, server_config) |
|
Yeah. Using a config struct has a potential advantage of easier mapping to settings. We could potentially have something like |
|
Changed user-facing config structs to be compositions: ClientConfig { conn: ConnectionConfig, ... }
ServerConfig { conn: ConnectionConfig, ... }this way we could easily provide some config for the shared |
Dedup client/server builders. And it's likely there will be more shared parameters in the future.