The Wayback Machine - https://web.archive.org/web/20190528152732/https://github.com/bluejekyll/trust-dns/issues/624
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

Expose send on the SyncClient #624

Open
mattias-p opened this issue Nov 21, 2018 · 2 comments

Comments

2 participants
@mattias-p
Copy link

commented Nov 21, 2018

I need to send queries with the RD flag disables. I started out with the SyncClient hello world example and just inlined my way towards the Message. However I can't inline client::ClientHandle::query because client::ClientResponse's data member is private and there's no constructor.

A public constructor for ClientResponse would solve my problem. Making the data member public would also work, but from my limited understanding it makes more sense to keep ClientResponse opaque.

@mattias-p mattias-p changed the title Private data member in client::ClientHandle Cannot construct client::ClientHandle Nov 21, 2018

@mattias-p mattias-p changed the title Cannot construct client::ClientHandle Cannot construct client::ClientResponse Nov 21, 2018

@bluejekyll

This comment has been minimized.

Copy link
Owner

commented Nov 22, 2018

I don't think this is actually what you want. Based on your description, you want to have more control over how the DnsRequest is constructed, or even directly send your own message. This method exists, but currently only exists on the ClientFuture, the async version of the SyncClient. It's the DnsHandle::send method implemented on BasicClientHandle (returned during construction of the ClientFuture). It should be fairly straightforward to expose this on the SyncClient in a similar way as the other methods, query for example, are exposed from the Client trait implementation.

Would you want to make this change to implement Client::send as a delegate to the underlying send method? This should look a lot like the other default implementations on the Client trait.

Another more extensive change we could make would be to create a new pre send hook, where a message could be inspected before send, and allow you to override different portions of the Message prior to it being sent. This might be better in the long run.

@mattias-p

This comment has been minimized.

Copy link
Author

commented Nov 22, 2018

Thank you so much for your excellent response! Your explanation validates my general approach and it helped me realize where I went wrong.

I've ended up with this rather nice method fn send(&self, message: Message) -> ClientResult<DnsResponse> (which is derived from client::Client::query). In this method I'm calling DnsHandle::send and I was wrapping its result in a ClientResponse (which is what client::ClientHandle::query ends up doing) before sending it on to tokio::runtime::Runtime::block_on (which is what client::Client::query does). I just didn't realize that I could replace runtime.block_on(ClientResponse(future)) with Ok(runtime.block_on(future)?).

I'm quite happy with that solution. For me getting my hands on the async stuff is pure gravy as I was planning to go that route anyway.

@bluejekyll bluejekyll changed the title Cannot construct client::ClientResponse Expose send on the SyncClient Jan 14, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.