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

feat: Add urls directly. #121

Merged
merged 4 commits into from
Nov 17, 2015
Merged

feat: Add urls directly. #121

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
9cdb0a2
feat: Add urls directly.
dignifiedquire Nov 16, 2015
1108c81
fix: Proper request headers for add url
dignifiedquire Nov 17, 2015
e7c4afe
fix: Do not catch user errors in the parser
dignifiedquire Nov 17, 2015
1dc719e
test: Add add stream test and some fixes
dignifiedquire Nov 17, 2015
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions src/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
var multiaddr = require('multiaddr')
var getConfig = require('./config')
var getRequestAPI = require('./request-api')
var request = require('request')
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure if request works in the browser. But the tests are testing that this actually works no?

Copy link
Contributor

Choose a reason for hiding this comment

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

Not being able to confirm, but read somewhere that request gets polifilled in browserify. Nevertheless, there is https://www.npmjs.com/package/browser-request for that :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@diasdavid @victorbjelkholm no need for all that, all tests are passing in the browser, request is 100% browser compatible :)


exports = module.exports = IpfsAPI

Expand Down Expand Up @@ -63,6 +64,10 @@ function IpfsAPI (host_or_multiaddr, port) {
opts = {}
}

if (typeof files === 'string' && files.startsWith('http')) {
files = request(files)
Copy link
Contributor

Choose a reason for hiding this comment

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

This sync call will block the loop (or am I missing some ES6 thing?)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The request is a stream object, so it will not block but will be piped into the new request effectively as far as I understand :)

Copy link
Contributor

Choose a reason for hiding this comment

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

`23:09 <+daviddias> dignifiedquire: interesting, so our thing understands 'strings', 'paths', 'buffers' and 'streams' too?``

Sweet!

Copy link
Contributor

Choose a reason for hiding this comment

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

Better add a test and docs for adding with a stream though, otherwise that might change in the future and then we don't realize what broke the add by url was actually the lack of add by stream

}

return requestAPI('add', null, opts, files, cb)
}

Expand Down
13 changes: 13 additions & 0 deletions test/tests.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -145,6 +145,19 @@ describe('IPFS Node.js API wrapper tests', function () {
}
})
})
it('adds a url', function (done) {
Copy link
Contributor

Choose a reason for hiding this comment

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

👍 for adding a test :D

this.timeout(10000)

var url = 'https://raw.githubusercontent.com/ipfs/js-ipfs-api/2a9cc63d7427353f2145af6b1a768a69e67c0588/README.md'
apiClients['a'].add(url, function (err, res) {
if (err) throw err

var added = res[0]

assert.equal(added.Hash, 'QmZmHgEX9baxUn3qMjsEXQzG6DyNcrVnwieQQTrpDdrFvt')
done()
})
})
})

describe('.cat', function () {
Expand Down