Skip to content
This repository has been archived by the owner on Aug 11, 2021. It is now read-only.

ipldResolver API as pullStreams #101

Closed
kumavis opened this issue Oct 17, 2017 · 6 comments
Closed

ipldResolver API as pullStreams #101

kumavis opened this issue Oct 17, 2017 · 6 comments
Labels
exp/expert Having worked on the specific codebase is important P4 Very low priority

Comments

@kumavis
Copy link
Contributor

kumavis commented Oct 17, 2017

would be nice to do a ipldResolver.get and get back a pullStream that passes back all nodes touched

would also be nice (for analytics) to be able to spy on all nodes being resolved by any request, something like a ipldResolver.createSpyStream()

cc @diasdavid

@kumavis
Copy link
Contributor Author

kumavis commented Oct 19, 2017

here are the events i used to make the ipld-resolver-viz
7a012d0

@daviddias
Copy link
Member

Making get return pull-stream sounds like a good idea to me, it matches ipfs.get/ipfs.files.get and prepares the API for the IPLD selector in the future.

I want to keep the API as simple as possible and let any extra porcelain in users land so that we do not increase the size of the codebase (=== tests, maintenance and bundle sizes) until it is completely obvious that it is a really good nice to have.

Following ipfs-inactive/interface-js-ipfs-core#126 (comment), I propose two API changes.

a) add an option to the .get options object to just return the final result (by default it should return all the touched objects)

b) expose two API calls, one that it is .get that returns an array with all the nodes touched and another one that it is .getPullStream that does the same but returns all the nodes touched in a pull-stream

As for the porcelain such as createSpyStream, you can in your codebase create a proxy for .get calls that keeps track of all objects traversed and returns on the SpyStream or you can monkey patch the IPFS node when to create it in your code.

@daviddias daviddias added status/ready Ready to be worked enhancement exp/expert Having worked on the specific codebase is important P1 High: Likely tackled by core team if no one steps up help wanted Seeking public contribution on this issue labels Jan 25, 2018
@zcstarr
Copy link

zcstarr commented Apr 2, 2018

@diasdavid just to be clear on this, I'd be replacing the existing get method? Trying to get a sense of what other projects I'd want to look at/ patch if this is the case.

@vmx vmx added P4 Very low priority and removed P1 High: Likely tackled by core team if no one steps up help wanted Seeking public contribution on this issue status/ready Ready to be worked labels Nov 14, 2018
@vmx
Copy link
Member

vmx commented Nov 14, 2018

With the Awesome Endeavour: Async Iterators we want to move away from pull streams. There's also an upcoming API review of all the IPLD stuff, so it doesn't make sense to spend more time on this for now.

@vmx vmx added the status/deferred Conscious decision to pause or backlog label Nov 14, 2018
@zcstarr
Copy link

zcstarr commented Nov 15, 2018 via email

@vmx
Copy link
Member

vmx commented May 8, 2019

We son't use pull-streams anymore, hence closing this issue.

@vmx vmx closed this as completed May 8, 2019
@ghost ghost removed the status/deferred Conscious decision to pause or backlog label May 8, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
exp/expert Having worked on the specific codebase is important P4 Very low priority
Projects
None yet
Development

No branches or pull requests

4 participants