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

Configuration simplification #202

Merged
merged 4 commits into from
Aug 23, 2024
Merged

Configuration simplification #202

merged 4 commits into from
Aug 23, 2024

Conversation

BiagioFesta
Copy link
Owner

This PR aims to simplify (and minimize) wtransport configurations (both server and client).

In particular, it gives the possibility to configure a server and client directly passing the underlying transport configuration (i.e., quinn). This follows the same approach adopted for the underlying TLS stack (i.e. rustls).

Some advanced methods are been removed, such as: token_key and enable_key_log in the spirit of the mission of wtransport: having a minimal and user-friendly API. However, providing (with a single method) the configuration via quinn and rustls all advanced configurations are still possible.

This PR also changes DnsResolver interface: it simplifies the code and avoid usage of a Mutex for DNS resolution

Copy link
Contributor

@finnbear finnbear left a comment

Choose a reason for hiding this comment

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

I successfully ported my app! The main side effect was needing to depend on quinn and quinn-proto, not just the latter.

@BiagioFesta
Copy link
Owner Author

wtransport re-export quinn: one can use wtransport::quinn:....

What about re-exporting quinn-proto as well?

@finnbear
Copy link
Contributor

finnbear commented Aug 23, 2024

wtransport re-export quinn: one can use wtransport::quinn:....

Cool, I missed that!

What about re-exporting quinn-proto as well?

Yes, please! (unless quinn could be convinced to have quinn::proto, allowing wtransport::quinn::proto 👀) Edit: no longer necessary!

@BiagioFesta
Copy link
Owner Author

BiagioFesta commented Aug 23, 2024

What is missing in quinn you'd consider for rexport? I see HandshakeTokenKey is already re-exported (as others)

wtransport/Cargo.toml Outdated Show resolved Hide resolved
Copy link
Contributor

@finnbear finnbear left a comment

Choose a reason for hiding this comment

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

Still working for me! (you can revert re-exporting quinn-proto)

@BiagioFesta
Copy link
Owner Author

Still working for me! (you can revert re-exporting quinn-proto)

Thank you for reviewing this and feedback.

Feel free to join the discord server so I can ping you some other time, for other reviews :P

@BiagioFesta BiagioFesta merged commit f7e809c into master Aug 23, 2024
16 checks passed
@BiagioFesta BiagioFesta deleted the config-from-quinn branch August 23, 2024 19:25
@finnbear
Copy link
Contributor

Feel free to join the discord server so I can ping you some other time, for other reviews :P

Amazing; didn't know that existed!

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