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

docs: add mfs stream ls methods #401

Merged
merged 1 commit into from
Dec 4, 2018
Merged

docs: add mfs stream ls methods #401

merged 1 commit into from
Dec 4, 2018

Conversation

achingbrain
Copy link
Collaborator

The only mfs.ls method at the moment buffers the output into an array before returning it to the user. This PR adds two new methods, lsPullStream and lsReadableStream to allow the user to either buffer the output themselves (in case they need sorting, etc) or just stream it on to an output of some sort.

@ghost ghost assigned achingbrain Nov 28, 2018
@ghost ghost added the in progress label Nov 28, 2018
achingbrain added a commit to ipfs-inactive/js-ipfs-mfs that referenced this pull request Nov 28, 2018
Implementation of ipfs-inactive/interface-js-ipfs-core#401

License: MIT
Signed-off-by: achingbrain <alex@achingbrain.net>
achingbrain added a commit to ipfs-inactive/js-ipfs-mfs that referenced this pull request Nov 28, 2018
Implementation of ipfs-inactive/interface-js-ipfs-core#401

License: MIT
Signed-off-by: achingbrain <alex@achingbrain.net>
Copy link
Contributor

@alanshaw alanshaw left a comment

Choose a reason for hiding this comment

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

Given that these are new APIs, do you think should they return cid (CID instance) in the response objects instead of hash (a string encoded CID)? Towards #394 and if we're serious about doing that then it would save us a breaking change down the line.

Do you think it would be confusing for developers if the objects returned from files.ls and files.ls*Stream are different shapes? I think yes, it probably is, but I could be persuaded.

@hugomrdias @daviddias do you have any strong opinions?

@hugomrdias
Copy link
Contributor

imo they should return the same as files.ls for now

@daviddias
Copy link
Contributor

I'm with @hugomrdias and @alanshaw

@achingbrain
Copy link
Collaborator Author

It think it's better for them to be consistent with the existing APIs, then we can break them all in one go down the line.

@alanshaw
Copy link
Contributor

Can we get some tests in here? Can we support it in ipfs-http-client with the new streaming ls in go-ipfs?

The only mfs ls method at the moment buffers the output into an array
before returning it to the user. This PR adds two new methods,
lsPullStream and lsReadableStream to allow the user to either buffer
the output themseleves (in case they need sorting, etc) or just
stream it on to an output of some sort.

N.b the http API will not actually do any streaming until ipfs/kubo#5611
is released.
@achingbrain
Copy link
Collaborator Author

Tests are added. ipfs-inactive/js-ipfs-http-client#903 is the other part of this.

@alanshaw alanshaw merged commit 55f303b into master Dec 4, 2018
@alanshaw alanshaw deleted the add-ls-streams branch December 4, 2018 15:59
@ghost ghost removed the in progress label Dec 4, 2018
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.

4 participants