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

feat: add files.ls streaming methods #903

Merged
merged 4 commits into from
Dec 5, 2018
Merged

feat: add files.ls streaming methods #903

merged 4 commits into from
Dec 5, 2018

Conversation

achingbrain
Copy link
Collaborator

N.b will not actually do any streaming until ipfs/kubo#5611 is released.

@ghost ghost assigned achingbrain Dec 4, 2018
@ghost ghost added the in progress label Dec 4, 2018
@achingbrain achingbrain requested a review from alanshaw December 4, 2018 15:07
@achingbrain achingbrain mentioned this pull request Dec 4, 2018
Merged
@achingbrain
feat: add files.ls streaming methods
c203ede 
N.b will not actually do any streaming until ipfs/kubo#5611
lands
@achingbrain achingbrain force-pushed the add-ls-streaming-methods branch from fc8c62a to c203ede Compare December 4, 2018 15:25
@achingbrain
Copy link
Collaborator Author

Actually, that go PR was for adding streaming to one of the other ls commands 😕

@achingbrain
Copy link
Collaborator Author

I've added a --stream option to the http endpoints for files.lsReadbleStream and files.lsPullStream that will actually the response as ndjson.

@achingbrain achingbrain mentioned this pull request Dec 4, 2018
Merged
chore: update interface-ipfs-core dependency
beb4c4c 
License: MIT
Signed-off-by: Alan Shaw <alan.shaw@protocol.ai>
@ghost ghost assigned alanshaw Dec 5, 2018
@alanshaw
Copy link
Contributor

alanshaw commented Dec 5, 2018

Currently getting these failures:

1) interface-ipfs-core tests
       .files.lsReadableStream
         should ls directory:
     Uncaught TypeError: stream.on is not a function
      at destroyer (node_modules/pump/index.js:26:10)
      at /Users/alan/Code/protocol-labs/js-ipfs-http-client/node_modules/pump/index.js:70:12
      at Array.map (<anonymous>)
      at pump (node_modules/pump/index.js:67:26)
      at send (src/files-mfs/ls-readable-stream.js:40:7)
      at f (node_modules/once/once.js:25:25)
      at streamToValue (src/utils/stream-to-json-value.js:30:5)
      at concat (src/utils/stream-to-value.js:12:22)
      at ConcatStream.<anonymous> (node_modules/concat-stream/index.js:37:43)
      at finishMaybe (node_modules/concat-stream/node_modules/readable-stream/lib/_stream_writable.js:630:14)
      at endWritable (node_modules/concat-stream/node_modules/readable-stream/lib/_stream_writable.js:638:3)
      at ConcatStream.Writable.end (node_modules/concat-stream/node_modules/readable-stream/lib/_stream_writable.js:594:41)
      at IncomingMessage.onend (_stream_readable.js:628:10)
      at endReadableNT (_stream_readable.js:1094:12)
      at process._tickCallback (internal/process/next_tick.js:63:19)

  2) interface-ipfs-core tests
       .files.lsReadableStream
         should ls -l directory:
     Uncaught TypeError: stream.on is not a function
      at destroyer (node_modules/pump/index.js:26:10)
      at /Users/alan/Code/protocol-labs/js-ipfs-http-client/node_modules/pump/index.js:70:12
      at Array.map (<anonymous>)
      at pump (node_modules/pump/index.js:67:26)
      at send (src/files-mfs/ls-readable-stream.js:40:7)
      at f (node_modules/once/once.js:25:25)
      at streamToValue (src/utils/stream-to-json-value.js:30:5)
      at concat (src/utils/stream-to-value.js:12:22)
      at ConcatStream.<anonymous> (node_modules/concat-stream/index.js:37:43)
      at finishMaybe (node_modules/concat-stream/node_modules/readable-stream/lib/_stream_writable.js:630:14)
      at endWritable (node_modules/concat-stream/node_modules/readable-stream/lib/_stream_writable.js:638:3)
      at ConcatStream.Writable.end (node_modules/concat-stream/node_modules/readable-stream/lib/_stream_writable.js:594:41)
      at IncomingMessage.onend (_stream_readable.js:628:10)
      at endReadableNT (_stream_readable.js:1094:12)
      at process._tickCallback (internal/process/next_tick.js:63:19)

  3) interface-ipfs-core tests
       .files.lsPullStream
         should ls directory:
     Uncaught TypeError: stream.on is not a function
      at destroyer (node_modules/pump/index.js:26:10)
      at /Users/alan/Code/protocol-labs/js-ipfs-http-client/node_modules/pump/index.js:70:12
      at Array.map (<anonymous>)
      at pump (node_modules/pump/index.js:67:26)
      at send (src/files-mfs/ls-readable-stream.js:40:7)
      at f (node_modules/once/once.js:25:25)
      at streamToValue (src/utils/stream-to-json-value.js:30:5)
      at concat (src/utils/stream-to-value.js:12:22)
      at ConcatStream.<anonymous> (node_modules/concat-stream/index.js:37:43)
      at finishMaybe (node_modules/concat-stream/node_modules/readable-stream/lib/_stream_writable.js:630:14)
      at endWritable (node_modules/concat-stream/node_modules/readable-stream/lib/_stream_writable.js:638:3)
      at ConcatStream.Writable.end (node_modules/concat-stream/node_modules/readable-stream/lib/_stream_writable.js:594:41)
      at IncomingMessage.onend (_stream_readable.js:628:10)
      at endReadableNT (_stream_readable.js:1094:12)
      at process._tickCallback (internal/process/next_tick.js:63:19)

  4) interface-ipfs-core tests
       .files.lsPullStream
         should ls -l directory:
     Uncaught TypeError: stream.on is not a function
      at destroyer (node_modules/pump/index.js:26:10)
      at /Users/alan/Code/protocol-labs/js-ipfs-http-client/node_modules/pump/index.js:70:12
      at Array.map (<anonymous>)
      at pump (node_modules/pump/index.js:67:26)
      at send (src/files-mfs/ls-readable-stream.js:40:7)
      at f (node_modules/once/once.js:25:25)
      at streamToValue (src/utils/stream-to-json-value.js:30:5)
      at concat (src/utils/stream-to-value.js:12:22)
      at ConcatStream.<anonymous> (node_modules/concat-stream/index.js:37:43)
      at finishMaybe (node_modules/concat-stream/node_modules/readable-stream/lib/_stream_writable.js:630:14)
      at endWritable (node_modules/concat-stream/node_modules/readable-stream/lib/_stream_writable.js:638:3)
      at ConcatStream.Writable.end (node_modules/concat-stream/node_modules/readable-stream/lib/_stream_writable.js:594:41)
      at IncomingMessage.onend (_stream_readable.js:628:10)
      at endReadableNT (_stream_readable.js:1094:12)
      at process._tickCallback (internal/process/next_tick.js:63:19)

@achingbrain
Copy link
Collaborator Author

I think you have the wrong version of a module somewhere. Are you running IPFS with ipfs/js-ipfs#1734 ? It won't have the streaming files.ls* methods otherwise.

@achingbrain
Copy link
Collaborator Author

After some discussion what you really wanted here was for the tests to be skipped since go-ipfs doesn't support streaming in mfs.ls type commands.

The tests are still useful for running with js-ipfs so it'd be a shame to disable them entirely. I gave up trying to figure out how to disable interface tests for certain implementations and instead changed the handlers to stream the responses if they are given a stream, otherwise to simulate the streaming on a complete response.

Be liberal in what you accept, etc.

@alanshaw
Copy link
Contributor

alanshaw commented Dec 5, 2018

After some discussion what you really wanted here was for the tests to be skipped since go-ipfs doesn't support streaming in mfs.ls type commands.

Yes, apologies for that. Although, I'm glad I didn't specify that now:

changed the handlers to stream the responses if they are given a stream, otherwise to simulate the streaming on a complete response.

🎉 now it "works" for everybody!

@alanshaw alanshaw merged commit 705855e into master Dec 5, 2018
@alanshaw alanshaw deleted the add-ls-streaming-methods branch December 5, 2018 13:33
@ghost ghost removed the in progress label Dec 5, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@alanshaw alanshaw Awaiting requested review from alanshaw

Assignees

@alanshaw alanshaw

@achingbrain achingbrain

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@achingbrain @alanshaw