-
Notifications
You must be signed in to change notification settings - Fork 75
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
Feature/dns resolver #80
Conversation
I'm using go-multiaddr-dns as a guide.
It's not just Travis :)
Don't want exceptions propagating out of here. We can handle the error case higher up. This is similar to the case where, for example, we request a dns6 resolution on a name which only has an ip4 address. If people need detailed reporting on DNS failures they can investigate afterwards
companion object { | ||
private val dnsProtocols = arrayOf(Protocol.DNS4, Protocol.DNS6, Protocol.DNSADDR) | ||
|
||
fun resolve(addr: Multiaddr, resolver: Resolver = DefaultResolver): List<Multiaddr> { |
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.
I would consider asynchronous invocation here (aka CompletableFuture
) since the DNS resolution may take a lot of time.
It also may make sense to check what non-blocking DNS resolve solutions exist to make it possible to resolve DNS in parallel without using a lot of threads
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.
This is something to think about, yes.
There is further work to do around DNS - the dnsaddr protocol, and also how we expose this as a configuration item (eg through the DSL) - so I'll come back and address that then.
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.
LGTM. A good example and flow of adding a protocol to multiaddr resolution.
Implements /dns4/hostname and /dns6/hostname resolution, resolving issue #70