Skip to content

Conversation

sirandreww-starkware
Copy link
Contributor

Description

Allow the users of the QUIC transport to control more QUIC configurations when they provide them.

Notes & open questions

  • Defaults change nothing.
  • Updated QUINN version and added QUINN-proto

Change checklist

  • I have performed a self-review of my own code
  • I have made corresponding changes to the documentation
  • I have added tests that prove my fix is effective or that my feature works
  • A changelog entry has been made in the appropriate crates

Copy link
Member

@elenaf9 elenaf9 left a comment

Choose a reason for hiding this comment

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

Thanks for the PR!

This adds a lot of new parameters that are all just quinn TransportConfig.
Wdyt of, instead of exposing each parameter separately, we have one function

pub fn with_transport_config<F: Fn(&mut quinn::TransportConfig)>(f: F) 

where users can modify the config directly?

We'd only need to be careful that there is no breaking change in quinn::TransportConfig when we upgrade the dependency. But I think it's easy to add a test for that.

@elenaf9
Copy link
Member

elenaf9 commented Oct 17, 2025

Thanks for the PR!

This adds a lot of new parameters that are all just quinn TransportConfig. Wdyt of, instead of exposing each parameter separately, we have one function

pub fn with_transport_config<F: Fn(&mut quinn::TransportConfig)>(f: F) 

where users can modify the config directly?

We'd only need to be careful that there is no breaking change in quinn::TransportConfig when we upgrade the dependency. But I think it's easy to add a test for that.

Discussed with @jxs out-of-band. We decided to not expose quinn::TransportConfig so we don't risk breaking our API when quinn::TransportConfig has a breaking change. So the current approach of this PR is fine.

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

Successfully merging this pull request may close these issues.

2 participants