Skip to content

Conversation

@gophry
Copy link

@gophry gophry commented Mar 8, 2016

Implement message body parsing for types #31 partial implementation. MX AND NS Records.

  1. function resolve() in Resolver Class now takes a type argument and defaults to A Record if none is passed.
  2. Query for MX and NS records returns an array.
  3. For MX queries priority is not captured in the current pull request.

@marcvdm
Copy link

marcvdm commented Mar 9, 2016

I was just trying to figure out why the parser did not give me back all the nameservers. Would be much appreciated if this was to be merged.

@gophry
Copy link
Author

gophry commented Mar 10, 2016

Feedback appreciated.
nameservers for igor.io, facebook.com, google.com, linode.com work perfectly as sample. cross checked against dig output too.

@clue
Copy link
Member

clue commented May 20, 2016

Thanks for the PR @gophry, I like this feature, but I'm not so sure about the current interface 👍

What does resolve() return for A records and how does this differ for other record types?

@gophry
Copy link
Author

gophry commented Jul 19, 2016

@clue apologies. i am under medical treatment for some time so was unavailable.
resolve() returns a string for A records and an array for NS and MX.

@clue
Copy link
Member

clue commented Jul 19, 2016

No worries, there's no need to hurry these things, I hope you're alright :)

resolve() returns a string for A records and an array for NS and MX.

What's the reasoning for this? I have to admit I'm not sold on the idea of using a single method that returns a different data structure depending on an input parameter. As an alternative, does it make sense to move this to separate methods instead?

@gophry
Copy link
Author

gophry commented Jul 20, 2016

I was in sense not comfortable myself with the idea. but the current resolve() returns a string. it will make sense to make them as separate methods.

@clue
Copy link
Member

clue commented Jul 21, 2016

FWIW, Node.js also has separate methods for each query type, for example https://nodejs.org/api/dns.html#dns_dns_resolvens_hostname_callback. Its resolve() method does in fact resolve with different types of values, I'm not sure this is something we want to do here.

@zainengineer
Copy link

any estimate on when it will be merged ?

@clue clue mentioned this pull request Jun 28, 2018
@jsor jsor closed this in #106 Jun 28, 2018
@clue
Copy link
Member

clue commented Jun 28, 2018

@gophry Thank you for your patience!

Support for NS records has been merged via #104.
Support for MX record has been merged via #106.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants