Skip to content
This repository has been archived by the owner on Apr 29, 2020. It is now read-only.

Added group dns #41

Merged
merged 4 commits into from
Mar 3, 2016
Merged

Added group dns #41

merged 4 commits into from
Mar 3, 2016

Conversation

RichardLitt
Copy link
Contributor

I was unable to get recursive working, and I'm not quite sure what it means or when it would be used. The example in the man pages seems to imply it is the default.

dignifiedquire
dignifiedquire reviewed Feb 17, 2016
View reviewed changes
apiary.apib
The following response expamles assume this DNS TXT record:

```
ipfs.io. TXT "dnslink=/ipfs/QmRzTuh2Lpuz7Gr39stNr6mTFdqAghsZec1JoUnfySUzcy ..."
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why is there a . after ipfs.io?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't know.

Copy link
Collaborator

Choose a reason for hiding this comment

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

@whyrusleeping @lgierth is this correct, or a typo?

Copy link

Choose a reason for hiding this comment

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

It doesn't do harm, but I'd remove it. With CNAME records the record value has to end in . though :) E.g. foobar.com CNAME gateway.ipfs.io..

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Weird. This is what is in the go-ipfs docs; should I PR those?

Copy link
Contributor

Choose a reason for hiding this comment

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

Sure, probably that was just copy paste

@dignifiedquire dignifiedquire mentioned this pull request Feb 17, 2016
Closed
@dignifiedquire
Copy link
Collaborator

Needs rebase

@RichardLitt
Copy link
Contributor Author

Will get to that when closer to merge.

@daviddias
Copy link
Contributor

I was unable to get recursive working, and I'm not quite sure what it means or when it would be used.

It is to follow till it finds an IPFS hash in a subdomain of the domain being looked up. The help menu has an example:

For example, with this DNS TXT record:

  ipfs.io. TXT "dnslink=/ipfs/QmRzTuh2Lpuz7Gr39stNr6mTFdqAghsZec1JoUnfySUzcy ..."

The resolver will give:

  > ipfs dns ipfs.io
  /ipfs/QmRzTuh2Lpuz7Gr39stNr6mTFdqAghsZec1JoUnfySUzcy

And with this DNS TXT record:

  ipfs.ipfs.io. TXT "dnslink=/dns/ipfs.io ..."

The resolver will give:

  > ipfs dns ipfs.io
  /dns/ipfs.io
  > ipfs dns --recursive
  /ipfs/QmRzTuh2Lpuz7Gr39stNr6mTFdqAghsZec1JoUnfySUzcy

RichardLitt added a commit to ipfs/kubo that referenced this pull request Mar 1, 2016
@RichardLitt
Remove period from dns text examples
7227386 
It is not useful. See ipfs-inactive/http-api-spec#41 (comment)

License: MIT
Signed-off-by: Richard Littauer <[email protected]>
@RichardLitt RichardLitt mentioned this pull request Mar 1, 2016
Merged
RichardLitt added a commit to ipfs/kubo that referenced this pull request Mar 1, 2016
@RichardLitt
Add default to dns resolve
8cce0aa 
See ipfs-inactive/http-api-spec#41 (comment)

License: MIT
Signed-off-by: Richard Littauer <[email protected]>
@RichardLitt RichardLitt mentioned this pull request Mar 1, 2016
Merged
@RichardLitt
Copy link
Contributor Author

@diasdavid how can I handily test recursive to produce a reliable output for the http-api?

@daviddias
Copy link
Contributor

@lgierth could you create a TXT record on ipfs.ipfs.io with dnslink=/dns/ipfs.io so that the example on the ipfs dns command works. Thank you :)

@daviddias daviddias assigned ghost and unassigned daviddias Mar 1, 2016
@dignifiedquire
Copy link
Collaborator

@RichardLitt needs rebase

@RichardLitt
Copy link
Contributor Author

@dignifiedquire I know, thanks. Because of the way the master file is structured, each will need a rebase. If I haven't merged it, it's because I'm waiting for something from the assigned person.

ghost
ghost reviewed Mar 3, 2016
View reviewed changes
apiary.apib
The following examples assume this DNS TXT record:

```
ipfs.io TXT "dnslink=/ipfs/QmRzTuh2Lpuz7Gr39stNr6mTFdqAghsZec1JoUnfySUzcy ..."
Copy link

Choose a reason for hiding this comment

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

The hash later on is a different one

Copy link

Choose a reason for hiding this comment

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

Maybe it'd be useful to use the dig command for DNS resolution? It'd give less experienced readers the tools they need in order to replicate what's being said. For example:

> dig +short TXT ipfs.io
dnslink=/ipfs/QmRzTuh2Lpuz7Gr39stNr6mTFdqAghsZec1JoUnfySUzcy

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not sure. I think adding in CLI commands might be confusing?

@ghost
Copy link

ghost commented Mar 3, 2016

This LGTM with one change:

  • hashes mismatch

The example works as is -- /dns/<domain-name>/ is not a thing anymore, the old help is severly outdated in this regard.

Sorry for blocking this so long :/

@RichardLitt
Added group dns
d0d89d0 
I was unable to get `recursive` working, and I'm not quite sure what it means or when it would be used. The example in the man pages seems to imply it is the default.
@RichardLitt
Remove .
c7358e8
@RichardLitt
Added Default false
872588f
@RichardLitt
Added recursive options examples
3c3a09b
@ghost
Copy link

ghost commented Mar 3, 2016

LGTM 👍

RichardLitt added a commit that referenced this pull request Mar 3, 2016
@RichardLitt
Merge pull request #41 from ipfs/feature/dns
440f034 
Added group `dns`
@RichardLitt RichardLitt merged commit 440f034 into master Mar 3, 2016
@RichardLitt
Copy link
Contributor Author

Thanks @lgierth! And no worries, wasn't blocked long.

@RichardLitt RichardLitt deleted the feature/dns branch March 3, 2016 16:27
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@RichardLitt @dignifiedquire @daviddias @whyrusleeping