ipld / js-ipld

The JavaScript Implementation of IPLD
https://ipld.io
MIT License
119 stars 37 forks source link

ipldResolver API as pullStreams #101

Closed kumavis closed 5 years ago

kumavis commented 7 years ago

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 commented 7 years ago

here are the events i used to make the ipld-resolver-viz https://github.com/ipld/js-ipld-resolver/commit/7a012d0b15d000406eed05a280a2cbc249be6fbd

daviddias commented 7 years ago

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 https://github.com/ipfs/interface-ipfs-core/issues/126#issuecomment-326991583, 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.

zcstarr commented 6 years ago

@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 commented 6 years ago

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.

zcstarr commented 6 years ago

Thanks for the heads up!

On Wed, Nov 14, 2018 at 5:30 AM Volker Mische notifications@github.com wrote:

With the Awesome Endeavour: Async Iterators https://github.com/ipfs/js-ipfs/issues/1670 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.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/ipld/js-ipld/issues/101#issuecomment-438661991, or mute the thread https://github.com/notifications/unsubscribe-auth/AAKkg7bOF5oIsfDQy-QBOsX9MQA5uSGQks5uvBrmgaJpZM4P8tuV .

vmx commented 5 years ago

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