Request for feeback: TRust-DNS::ClientFuture

I just landed the the Client side Futures implementations for TRust-DNS into master. It's a pretty easy conversion from the old client:

let result = client.query(record.get_name(), record.get_dns_class(), RecordType::A).unwrap()

the new

let io_loop = tokio_core::reactor::Core::new().unwrap();
let result = io_loop.run(client.query(new.get_name(), new.get_dns_class(), RecordType::A, false)).unwrap().unwrap()

The difference here is that in the original client Mio was being used internally, not giving any control over the event loop to the caller. Now the caller is responsible for the event loop. The extra unwrap() is for the result of the Oneshot completion future, and then the actual ClientResult which matches the original. This should allow the ClientFuture to be used in any Futures context and be compatible with other peoples Tokio based tools, which was the entire point of this month+ rewrite :wink:

The ClientFuture::new() also takes an optional Signer. This is used right now for SIG0, which is the only auth method supported by TRust-DNS for dynamic DNS updates. I've been considering making a separate type for an UpdateClient and a standard QueryClient, though I haven't figured out a clean way to do that yet.

The best examples I have for usage right now are in the tests:

https://github.com/bluejekyll/trust-dns/blob/master/src/client/client_future.rs#L938

I want to especially thank @alexcrichton for reviewing a lot of this code. He had many good pointers and explanations that helped me get this done. I think I incorporated all of those suggestions in :slight_smile:

I may entirely remove the old Client, but it's still there and works. The other option is to keep it as a facade over this implementation, presenting a simple blocking client.

2 Likes

For anyone following, I've got the initial interface for SecureClientFuture laid out: https://github.com/bluejekyll/trust-dns/blob/bfry/future_dnssec/src/client/secure_client_future.rs

What's cool is that this is making use of the composability of futures.

As an example, here is the ClientFuture usage:

    let mut io_loop = Core::new().unwrap();
    let addr: SocketAddr = ("8.8.8.8",53).to_socket_addrs().unwrap().next().unwrap();
    let (stream, sender) = UdpClientStream::new(addr, io_loop.handle());
    let client = ClientFuture::new(stream, sender, io_loop.handle(), None);

    io_loop.run(test_query(&client)).unwrap();

And then here is a SecureClientFuture example:

    let mut io_loop = Core::new().unwrap();
    let addr: SocketAddr = ("8.8.8.8",53).to_socket_addrs().unwrap().next().unwrap();
    let (stream, sender) = UdpClientStream::new(addr, io_loop.handle());
    let client = ClientFuture::new(stream, sender, io_loop.handle(), None);
    let secure_client = SecureClientHandle::new(client);

    test_secure_query_example(secure_client, io_loop);

It's just an additional wrap and then the exact same usage. This is very different from the non-future implementation which used a completely separate function to perform secure lookups. This is a WIP, so it hasn't landed in master yet.

I've made a lot of progress over the last week on the SecureClientFuture. I'll have that pushed soon. I have a question up, that if anyone wants to provide opinions, I'd definitely appreciate them, basically I'm considering changing the result from query() from Message to Vec<Record> of possibly just the query fields.

Feedback appreciated, either here on on the issue:

https://github.com/bluejekyll/trust-dns/issues/51

FYI, the SecureClientHandle, which is basically a wrapper around the ClientHandle returned by the ClientFuture just landed in master.

It took me a lot longer to get this done than I expected, but the rewrite of the DNSSec proof chain from the original Client to the a futures based model is complete. Now I get to move onto migrating the Server to futures-rs.

https://github.com/bluejekyll/trust-dns/commit/1e50a8b904d918c4b26e487e3848b1bbe98e025a