-
Notifications
You must be signed in to change notification settings - Fork 74
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
add builder interface for making servers #1122
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks good.
What's your thinking on if/when we replace the old interface?
As usual for non-trivial interfaces changes, even though this is not a breaking one, I like to try it on Omicron first. That worked out pretty well: All of these were pretty trivial to convert and all of them immediately called |
Windows CI is failing: This appears to be steffengy/schannel-rs#106. We're getting
One of those is hyper-rustls, which we're a little behind on (see #1110) and it's possible updating that may help? For now though I'm going to disable this test on Windows because this isn't trying to verify TLS functionality (we do that elsewhere) -- just that this constructor does successfully start TLS. |
Despite my threat to just disable this test on Windows, the lingering fear of papering over something important keeps me from doing it. I was able to work around the panic by updating seanmonstar/reqwest#826 It seems possible that the certificate being generated by I'm going to try using rustls-tls (this is just for our own tests) to see if that helps. |
Split from #1120. Depends on #1121.
This PR adds a new builder interface for constructing servers. I changed a few of the examples and tests to use it, but not all of them yet in case we want to iterate on the interface in review.