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

Large change to resolve URLs with paths or versions. #4

Merged
merged 4 commits into from
Dec 12, 2017

Conversation

millette
Copy link
Contributor

Allows it to resolve:

  • hostname.com/path
  • hostname.com+4
  • dat://hostname.com/path
  • a1b[...]3c4/path (64 char key)
  • a1b[...]3c4+4
  • dat://a1b[...]3c4/path

Depends on dat-ecosystem-archive/dat-encoding#23

millette/dat-shell#5 depends on these changes.

@millette
Copy link
Contributor Author

The travis result is unexpected, I should test locally. It's working with node 6 (I didn't think it would) but fails as expected on node 4. As mentionned above, it depends on dat-ecosystem-archive/dat-encoding#23

At home, I was only testing on node 8.

@millette
Copy link
Contributor Author

millette commented Dec 11, 2017

dat-encoding 5.0.1 was published. Running the tests again locally with node 4.8.7, 6.12.2, 8.9.3 and 9.2.1 and all tests pass.

@joehand
Copy link
Collaborator

joehand commented Dec 11, 2017

I think this changes the DNS resolution to last. Is there a reason for that? (forgot to add update readme and add that as first in resolution order)

Re-ran travis and tests pass. Yay. We should add 8 here too though.

@millette
Copy link
Contributor Author

millette commented Dec 11, 2017

The order is:

  1. stringKey
  2. Headers
  3. json .key
  4. dat-dns

Note that we skip tests 2 and 3 when either nets returns an error or the statusCode !== 200.

dat-dns happens last since if we ask for example.com/a-dat it may return a specific key at those headers or json (with dat .key). If there's nothing there, we do dat-dns for a more generic key.

In the readme, I can add the 4th step which is currently missing (dat-dns).

I can also add node 8 to travis in this patch or another if you prefer.

@joehand joehand merged commit 4b376e1 into dat-ecosystem-archive:master Dec 12, 2017
@joehand
Copy link
Collaborator

joehand commented Dec 12, 2017

Thanks for getting through all this! Published as 2.1.0

@millette
Copy link
Contributor Author

You're very welcome :-)

@millette millette deleted the more-tests branch December 12, 2017 01:16
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.

2 participants