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

Settings #10

Open
stammw opened this issue Aug 6, 2020 · 3 comments
Open

Settings #10

stammw opened this issue Aug 6, 2020 · 3 comments

Comments

@stammw
Copy link
Contributor

stammw commented Aug 6, 2020

@seanmonstar's proposal contains a builder API for connection that would accept the few settings available defined by HTTP/3, and QPACK:

Connection::builder().max_field_section_size(1024 * 64).handshake(quic_connection)

While this is clearly an ergonomic advantage for the user, I think we should also have a Settings type:

  • This makes it possible to reuse the same settings for many connections.
  • Yes ConnecitonBuilder::handshake() could take &self instead of self
  • But maybe we'd like to be sure we're in possession of a valid Settings (of which parameters need to be bound-checked).
  • Having a type for that is an opportunity to "encapsulate" bound checks and default values.

The QPACK settings are not to be neglected even if the plan is not to integrate QPACK for now because it blocks streams: Headers are always encoded with QPACK, what changes when disabling it is that there is no dynamic table in the encoding game. This configuration is linked to qpack's SETTINGS_QPACK_MAX_TABLE_CAPACITY: a value of 0 means this is disabled.

The Settings type, available from a builder-pattern thing, can be passed when building a connection, like so:

let settings = Settings::builder()
    .max_field_section_size(1024 * 64)
    .qpack_max_table_capacity(4096)
    .build()?;
  
Connection::builder().settings(settings).handshake(server1)
Connection::builder().settings(settings).handshake(server2)

I've got a similar type in quinn_h3::Settings, but the ergonomics are not that great.

@jxs
Copy link
Contributor

jxs commented Aug 6, 2020

following the proposal:

A user could create their QUIC implementation’s concept of a Connection, and then pass that to h3. This is a similar design to h2, which asks users to do any transport building before handing it to server::handshake(conn) or client::handshake(conn).

h2::server::Builder::handshake takes &self which is cloned inside and can therefore be used for multiple connections.
Can't this be achieved with a similar api to h2?

let builder = Connection::builder()
    .max_field_section_size(1024 * 64)
    .qpack_max_table_capacity(4096);
  
let srv1 = builder.handshake(server1)
let srv2 = builder.handshake(server2)

@seanmonstar
Copy link
Member

Yea, like @jxs mentioned, the builder in h2 basically is the "settings" type, and we could do similar here.

@stammw
Copy link
Contributor Author

stammw commented Aug 7, 2020

Okay, that's fine to do as it's done in h2.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants