Skip to content
This repository has been archived by the owner on Oct 23, 2022. It is now read-only.

Implement /dns and /resolve #353

Merged
merged 9 commits into from
Sep 8, 2020
Merged

Implement /dns and /resolve #353

merged 9 commits into from
Sep 8, 2020

Conversation

ljedrz
Copy link
Member

@ljedrz ljedrz commented Sep 1, 2020

Add a rudimentary implementation of the /dns and /resolve endpoints; putting it out there already, as due to the similarity of these two endpoints I'm not 100% sure how much we want to "condense" their inner workings.

This upgrades or conformance suite stats from

170 passing
50 pending

to

178 passing
42 pending

@ljedrz ljedrz requested a review from koivunej September 1, 2020 11:59
@@ -18,7 +18,7 @@ use thiserror::Error;
#[error("no dnslink entry")]
pub struct DnsLinkError;

type FutureAnswer = Pin<Box<dyn Future<Output = Result<Answer, io::Error>>>>;
Copy link
Member Author

@ljedrz ljedrz Sep 1, 2020

Choose a reason for hiding this comment

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

note: this one gave me a bit of a headache - the compiler was pointing to a very long issue with the HTTP warp call, but this was actually the root cause of the problem

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yeah I wonder if the future should be just written open as async fn to be simpler in every way, but haven't tried this myself ... for a while at least. Though, there's nothing obvious causing this, perhaps the path inside domain-resolve is just so long.

@ljedrz
Copy link
Member Author

ljedrz commented Sep 1, 2020

seems that I've missed a test - I'll leave this one here as a draft and tinker with it some more

@ljedrz ljedrz marked this pull request as draft September 1, 2020 12:07
Signed-off-by: ljedrz <ljedrz@gmail.com>
Signed-off-by: ljedrz <ljedrz@gmail.com>
… /dns

Signed-off-by: ljedrz <ljedrz@gmail.com>
Signed-off-by: ljedrz <ljedrz@gmail.com>
@ljedrz
Copy link
Member Author

ljedrz commented Sep 2, 2020

the curious thing is that only the /dns tests fail on Windows, while the /resolve ones pass, even though they both use the same internal machinery 🤔; why does Windows not resolve ipfs.io?

Signed-off-by: ljedrz <ljedrz@gmail.com>
Signed-off-by: ljedrz <ljedrz@gmail.com>
Signed-off-by: ljedrz <ljedrz@gmail.com>
src/lib.rs Show resolved Hide resolved
src/lib.rs Outdated Show resolved Hide resolved
Signed-off-by: ljedrz <ljedrz@gmail.com>
Signed-off-by: ljedrz <ljedrz@gmail.com>
@ljedrz ljedrz marked this pull request as ready for review September 8, 2020 13:49
Copy link
Contributor

@aphelionz aphelionz left a comment

Choose a reason for hiding this comment

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

bors r+

@ljedrz
Copy link
Member Author

ljedrz commented Sep 8, 2020

in the end the issue was that the domain_resolv crate just doesn't read the DNS configuration in Windows; the docs for ResolvConf::default (which is used by the StubResolver) state:

XXX This currently only works for Unix-y systems.

the solution was to use the ipconfig crate in order to read the DNS configuration in Windows and to provide that information to the StubResolver manually.

@ljedrz
Copy link
Member Author

ljedrz commented Sep 8, 2020

Cc #6

@ljedrz
Copy link
Member Author

ljedrz commented Sep 8, 2020

bors r-

@bors
Copy link
Contributor

bors bot commented Sep 8, 2020

Canceled.

@ljedrz
Copy link
Member Author

ljedrz commented Sep 8, 2020

bors r+

@bors
Copy link
Contributor

bors bot commented Sep 8, 2020

Build succeeded:

@bors bors bot merged commit 0019ece into rs-ipfs:master Sep 8, 2020
@ljedrz ljedrz deleted the dns_resolve branch September 8, 2020 15:26
@koivunej koivunej mentioned this pull request Sep 22, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants