Adds API for ipfs.files.get. #28

hackergrrl · 2016-06-07T16:06:27Z

daviddias · 2016-06-20T17:59:41Z

src/files.js

@@ -278,8 +275,7 @@ module.exports = (common) => {
          const hash = new Buffer(bs58.decode('QmPZ9gcCEpqKTo6aq61g2nXGUhM4iCL3ewB6LDXZCtioEB'))
          ipfs.cat(hash)
            .then((stream) => {
-              stream.pipe(bl((err, bldata) => {
-                expect(err).to.not.exist
+              stream.pipe(concat((bldata) => {


bl was nice cause it also catched any error

The stream will still throw, which isn't as nice as the explicit handling.

hackergrrl · 2016-06-23T19:21:33Z

Let me know if LGTM and I'll squash for ya! @diasdavid

daviddias · 2016-06-28T08:35:01Z

Merged the js-ipfs-unixfs-engine PR.

Are these the ones missing on js-ipfs-api and js-ipfs to get the tests from files.get working there?

Let's squash these, get js-ipfs ready, both passing the tests and put a ✔️ on this :)

daviddias · 2016-06-29T21:04:47Z

@noffle I believe I unlocked you by merging and releasing the unixfs-engine PR. Let me know if you need anything else from me :)

daviddias · 2016-08-04T14:39:44Z

@noffle this is looking good. Could you squash the commits for the merge and also, follow ipfs-inactive/js-ipfs-unixfs-engine#52 (comment) so that PR gets merged too.

Excited to have the get command finally in too :)

daviddias · 2016-08-05T13:54:41Z

src/files.js

+          stream.pipe(concat((files) => {
+            expect(err).to.not.exist
+            expect(files).to.be.length(1)
+            expect(files[0].path).to.equal(hash)


Seems that here you expect that the path is a string b58 encoded multihash, while... ->

The correct thing to do is to check for the bs58-encoded value. If you're seeing anything else it's definitely an error to accommodate pre-this-PR. Thanks for noticing.

daviddias · 2016-08-05T14:05:03Z

There are some lint errors, but I guess that is our bad for not having added CLI just to test that.

/Users/david/code/interface-ipfs-core/src/files.js
    9:7    error  'isNode' is defined but never used  no-unused-vars
   29:101  error  Unexpected trailing comma           comma-dangle
  367:70   error  Unexpected trailing comma           comma-dangle
  384:41   error  Unexpected trailing comma           comma-dangle

Adding CI now

daviddias · 2016-08-05T14:05:21Z

@noffle got ipfs-inactive/js-ipfs-http-client#296 (comment), which makes me think something is weird with js-ipfs since it doesn't error.

hackergrrl · 2016-08-06T00:18:52Z

The multihash buffer vs bs58-encoded string is an old breakage; forgot to push the fix. This was what I addressed in js-ipfs-unixfs-engine a while back.

Update: worse! Somehow my commits got merged twice on this branch -- what a mess. I went back and re-rebased my way to a much cleaner state. Tests pass now for me, commits are squashed, linting is done! 👌

daviddias · 2016-08-06T15:26:45Z

src/files.js


 module.exports = (common) => {
-  describe('.files', () => {
+  describe.only('.files', () => {


Will need to change this. But I can also do it on merge :)

Agh, I'm guilty of continually doing this. Will correct. :)

daviddias · 2016-08-06T15:56:04Z

Thank you! I got an error in js-ipfs-api browser tests though, see: https://github.com/ipfs/js-ipfs-api/pull/296/files#r73787506

Also, in js-ipfs, I'm now getting:

  1) core --both .files .get with a multihash:
     Uncaught AssertionError: expected <Buffer 12 20 12 0f 6a f6 01 d4 6e 10 b2 d2 e1 1e d7 1c 55 d2 5f 30 42 c2 25 01 e4 1d 12 46 e7 a1 e9 d3 d8 ec> to deeply equal 'QmPZ9gcCEpqKTo6aq61g2nXGUhM4iCL3ewB6LDXZCtioEB'

daviddias · 2016-08-09T13:20:13Z

README.md

@@ -20,6 +20,7 @@
  - [Files](/API/files)
    - [`add`](/API/files#add)
    - [`createAddStream`](/files#createaddstream)
+    - [`get`](/API/files#get)


@noffle seems that this was never added to the files.md. Maybe a missing commit?

Not sure where it got dropped. Thanks for noticing! I'll re-add.

hackergrrl · 2016-08-09T16:36:20Z

Closing this in favour of #54.

hackergrrl force-pushed the ipfs-get branch from fb11b8a to e35b4a2 Compare June 7, 2016 16:26

hackergrrl mentioned this pull request Jun 15, 2016

Add files.get command and tests ipfs-inactive/js-ipfs-http-client#296

Closed

daviddias reviewed Jun 20, 2016
View reviewed changes

hackergrrl force-pushed the ipfs-get branch from 520f544 to 20f5218 Compare June 23, 2016 19:21

daviddias mentioned this pull request Jun 30, 2016

next generation ipfs-inactive/js-ipfs-http-client#305

Merged

65 tasks

hackergrrl force-pushed the ipfs-get branch 2 times, most recently from 07539d6 to a58ac3c Compare July 14, 2016 21:45

daviddias reviewed Aug 5, 2016
View reviewed changes

Adds API for ipfs.files.get.

a86753a

hackergrrl force-pushed the ipfs-get branch 2 times, most recently from 6ae5088 to e656e7c Compare August 5, 2016 23:44

Add ipfs.files.get tests.

2e94ac9

hackergrrl force-pushed the ipfs-get branch from e656e7c to 2e94ac9 Compare August 6, 2016 00:17

daviddias reviewed Aug 6, 2016
View reviewed changes

daviddias assigned hackergrrl Aug 6, 2016

daviddias mentioned this pull request Aug 6, 2016

proposal: Files API - dot return DAGNodes on Add #29

Closed

daviddias unassigned hackergrrl Aug 6, 2016

daviddias added the milestone 2 label Aug 6, 2016

daviddias assigned hackergrrl Aug 7, 2016

This was referenced Aug 8, 2016

Sprint: Aug 1 → Aug 8 ipfs/team-mgmt#130

Closed

feat/ipfs.files.get #54

Merged

daviddias reviewed Aug 9, 2016
View reviewed changes

hackergrrl closed this Aug 9, 2016

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adds API for ipfs.files.get. #28

Adds API for ipfs.files.get. #28

hackergrrl commented Jun 7, 2016 •

edited by daviddias

Loading

daviddias Jun 20, 2016

hackergrrl Jun 23, 2016

hackergrrl commented Jun 23, 2016

daviddias commented Jun 28, 2016 •

edited

Loading

daviddias commented Jun 29, 2016

daviddias commented Aug 4, 2016

daviddias Aug 5, 2016

hackergrrl Aug 9, 2016

daviddias commented Aug 5, 2016

daviddias commented Aug 5, 2016

hackergrrl commented Aug 6, 2016

daviddias Aug 6, 2016

hackergrrl Aug 9, 2016

daviddias commented Aug 6, 2016

daviddias Aug 9, 2016

hackergrrl Aug 9, 2016

hackergrrl commented Aug 9, 2016

Adds API for ipfs.files.get. #28

Adds API for ipfs.files.get. #28

Conversation

hackergrrl commented Jun 7, 2016 • edited by daviddias Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hackergrrl commented Jun 23, 2016

daviddias commented Jun 28, 2016 • edited Loading

daviddias commented Jun 29, 2016

daviddias commented Aug 4, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

daviddias commented Aug 5, 2016

daviddias commented Aug 5, 2016

hackergrrl commented Aug 6, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

daviddias commented Aug 6, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hackergrrl commented Aug 9, 2016

hackergrrl commented Jun 7, 2016 •

edited by daviddias

Loading

daviddias commented Jun 28, 2016 •

edited

Loading