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

std::future preview #849

Merged
merged 24 commits into from Oct 18, 2019
Merged

std::future preview #849

merged 24 commits into from Oct 18, 2019

Conversation

bluejekyll
Copy link
Member

@bluejekyll bluejekyll commented Aug 15, 2019

I was hoping this could be done without pulling a dependency on async/await, but no such luck. There are some complex areas that make this painful.

@gfreezy
Copy link

gfreezy commented Aug 26, 2019

What's the progress of this PR? I want to do some help.

@bluejekyll
Copy link
Member Author

FYI: Current progress. the base proto, client, and resolver crates are all fully passing tests with default features.

Now working on server, *tls, and the integration test crates. No interfaces have been updated yet to be async/await friendly, but getting there.

@baloo
Copy link

baloo commented Oct 7, 2019

There is a weird file named "1" added by 6602c68, looks like an error.

@bluejekyll
Copy link
Member Author

Oh, huh. I wonder how that happened. Thanks for the heads up.

@bluejekyll
Copy link
Member Author

FYI, for an update on the progress of this. All crates are compiling with default features, and integration tests are all passing, except for one. There is a test that was reliant on task local storage for tracking the depth of CNAME chain lookups in the resolver. I'm working to rewrite that logic such that we can still make sure we don't follow CNAME chains beyond a reasonable level.

@bluejekyll
Copy link
Member Author

Ok, all default features are now compiling, all tests passing. Max Query depth for CNAME chains is enforced again. This is getting close for a review.

@bluejekyll
Copy link
Member Author

FYI, just push a rebase onto master. So any forks should be aware. I'm going to go through and review all of this change, I there are quite a few FIXMEs I sprinkled throughout, need to clean that up.

crates/proto/src/udp/udp_stream.rs Show resolved Hide resolved
crates/proto/src/xfer/dns_multiplexer.rs Show resolved Hide resolved
crates/https/src/https_client_stream.rs Outdated Show resolved Hide resolved
crates/https/src/https_client_stream.rs Outdated Show resolved Hide resolved
crates/native-tls/src/tls_stream.rs Outdated Show resolved Hide resolved
crates/proto/src/udp/udp_client_stream.rs Outdated Show resolved Hide resolved
crates/proto/src/udp/udp_client_stream.rs Outdated Show resolved Hide resolved
crates/resolver/src/system_conf/unix.rs Outdated Show resolved Hide resolved
HandleRequest::Result(Ok(_)) => Ok(Async::Ready(())),
HandleRequest::Result(Err(res)) => {
// TODO: return ()
type Output = Result<(), ()>;
Copy link
Member Author

Choose a reason for hiding this comment

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

Convert to ()

type Item = ();
type Error = ();
impl<R: ResponseHandler + Unpin> Future for LookupFuture<R> {
// TODO: return ()
Copy link
Member Author

Choose a reason for hiding this comment

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

convert to ()

proto types all updated

all types updated to new tokio and futures

tcp_stream compiling

wip: most types updated

tcp stream updated to std::future

proto crate compiling, need to fix some udp apis

proto compiling and all tests passing

mdns and secure socket handle compiling
all types compiling

cleanup some usage of the pins

all default feature tests passing for resolver!
server and tests compiling, wip: need tests to pass

fix server runtime, all default-features passing
https_client_stream compiling

https passing all tests
all tests passing but chained
@bluejekyll bluejekyll marked this pull request as ready for review October 16, 2019 16:43
@bluejekyll bluejekyll changed the title (WIP) std::future preview std::future preview Oct 16, 2019
@bluejekyll
Copy link
Member Author

Ok. I believe this is ready to go. I’ll be merging in then will publish the first preview! We’ll want to adjust some of the APIss I’m sure.

@zonyitoo
Copy link
Contributor

Still cannot pass travis' tests.

@bluejekyll
Copy link
Member Author

Yes. I think those are my last cleanup changes. They were passing just before this.

@baloo
Copy link

baloo commented Oct 18, 2019

@bluejekyll congrats! and thanks for your work.

@bluejekyll
Copy link
Member Author

Thanks, I'm just happy all the Futures work stabilized, so there shouldn't be a need to do a massive upgrade like this again. That was a slog.

@bluejekyll bluejekyll deleted the std-future-preview branch October 28, 2019 16:30
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.

None yet

4 participants