-
Notifications
You must be signed in to change notification settings - Fork 298
Conversation
80cda06
to
a02bf45
Compare
The interface tests in ipfs-inactive/js-ipfs-http-client#1183 have started throwing `UnhandledPromiseRejection`s for a connection refused error to `/api/v0/shutdown`. This is because `stop` was being called and then called again in `killProcess`. With callbacks this error was smothered but it is now appearing with the promise-only API.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Beautiful ❤️
I'm going to back out of the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@alanshaw after sleeping on it I am bit worried about removing buffered versions of commands like cat
.
Instead of
const data = await ipfs.cat(cid)
user now needs to ALWAYS wrap it with a helper:
const data = await concat(ipfs.cat(cid))
Something feels off here 🤔
I would expect methods like cat
to be easy to use, buffered versions (as they are now) and provide separatecatStream
for cases where streaming is needed.
Was this discussed before? Did I miss the train?
@@ -26,13 +25,22 @@ module.exports = configure(({ ky }) => { | |||
}) | |||
|
|||
for await (let message of ndjson(toIterable(res.body))) { | |||
// 3 = QueryError |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it possible to extract some of this boilerplate
for await (let message of ndjson(toIterable(res.body)))
if (message.Type === 3)
- etc
into a function / class?
} | ||
}) | ||
|
||
function toCoreInterface (entry) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
May be worth extracting this into a shared function also
If they want to buffer data into memory then yes that's what they need to do. They definitely don't ALWAYS need to do this! I'd advise against doing so anyway unless you know how much data you're buffering and that it's coming from your local repo. If we provide them, users will just use the non-streaming APIs and be frustrated when things take a long time to yield any results and they get no feedback. For big files, it's necessary to stream data and given the majority of people won't know the size of the file they're retrieving, and are trying to obtain it from a network of poorly connected peers we should provide an API that a) encourages them to do the right thing and not buffer content into memory regardless of the size of the file they're retrieving and b) reduces time to first byte, gives them visibility over progress and allows them to differentiate between the file being unavailable or it being large. On a related point, there's likely to be built in helpers to async iterables soon https://github.com/tc39/proposal-iterator-helpers so you'd be able to |
a83f101
to
2aaf632
Compare
ae85388
to
bd5f0ae
Compare
49d8e34
to
08425af
Compare
08425af
to
a27302d
Compare
TLDR;
peer-info
andpeer-id
Now that internals are all async/await/iterables and
fetch
the next step is to bubble up that goodness to the core API, removing the multiple stream APIs and removing callback support.I'm also proposing removing
peer-info
andpeer-id
, since these drastically increase the bundle size by pulling inlibp2p-crypto
, for which 99% of it's capability is unused. In place ofpeer-id
we return aCID
, which can easily be converted to aPeerId
instance via:In place of
peer-info
we return an object{ id: CID, addrs: Multiaddr[] }
, which can easily be converted to aPeerInfo
like:refs ipfs/js-ipfs#1670
refs ipfs/js-ipfs#2611
refs ipfs-inactive/interface-js-ipfs-core#394
TODO:
addFromFs
andaddFromUrl
and exportglobSource
andurlSource
insteadinterface-ipfs-core
testsinterface-ipfs-core
Depends on:
BREAKING CHANGE: Callbacks are no longer supported on any API methods. Please use a utility such as
callbackify
on API methods that return Promises to emulate previous behaviour.BREAKING CHANGE:
PeerId
andPeerInfo
classes are no longer statically exported fromipfs-http-client
since they are no longer used internally.BREAKING CHANGE:
pin.add
results now contain acid
property (a CID instance) instead of a stringhash
property.BREAKING CHANGE:
pin.ls
now returns an async iterable.BREAKING CHANGE:
pin.ls
results now contain acid
property (a CID instance) instead of a stringhash
property.BREAKING CHANGE:
pin.rm
results now contain acid
property (a CID instance) instead of a stringhash
property.BREAKING CHANGE:
add
now returns an async iterable.BREAKING CHANGE:
add
results now contain acid
property (a CID instance) instead of a stringhash
property.BREAKING CHANGE:
addReadableStream
,addPullStream
have been removed.BREAKING CHANGE:
ls
now returns an async iterable.BREAKING CHANGE:
ls
results now contain acid
property (whose value is a CID instance) instead of ahash
property.BREAKING CHANGE:
files.ls
now returns an async iterable.BREAKING CHANGE:
files.readPullStream
andfiles.readReadableStream
have been removed.BREAKING CHANGE:
files.read
now returns an async iterable.BREAKING CHANGE:
files.lsPullStream
andfiles.lsReadableStream
have been removed.BREAKING CHANGE:
files.ls
now returns an async iterable.BREAKING CHANGE:
files.ls
results now contain acid
property (whose value is a CID instance) instead of ahash
property.BREAKING CHANGE:
files.ls
no longer takes along
option (in core) - you will receive all data by default.BREAKING CHANGE:
files.stat
result now contains acid
property (whose value is a CID instance) instead of ahash
property.BREAKING CHANGE:
get
now returns an async iterable. Thecontent
property value for objects yielded from the iterator is now an async iterable that yieldsBufferList
objects.BREAKING CHANGE:
stats.bw
now returns an async iterable.BREAKING CHANGE:
addFromStream
has been removed. Useadd
instead.BREAKING CHANGE:
isIPFS
is no longer exported from the client, pleasenpm i is-ipfs
or include the CDN script tag<script src="https://unpkg.com/is-ipfs/dist/index.min.js"></script>
to use this utility in your applications.BREAKING CHANGE:
addFromFs
has been removed. Please use the exportedglobSource
utility and pass the result toadd
. See the glob source documentation for more details and an example.BREAKING CHANGE:
addFromURL
has been removed. Please use the exportedurlSource
utility and pass the result toadd
. See the URL source documentation for more details and an example.BREAKING CHANGE:
name.resolve
now returns an async iterable. It yields increasingly more accurate resolved values as they are discovered until the best value is selected from the quorum of 16. The "best" resolved value is the last item yielded from the iterator. If you are interested only in this best value you could useit-last
to extract it like so:BREAKING CHANGE:
block.rm
now returns an async iterable.BREAKING CHANGE:
block.rm
now yields objects of{ cid: CID, error: Error }
.BREAKING CHANGE:
dht.findProvs
,dht.provide
,dht.put
anddht.query
now all return an async iterable.BREAKING CHANGE:
dht.findPeer
,dht.findProvs
,dht.provide
,dht.put
anddht.query
now yield/return an object{ id: CID, addrs: Multiaddr[] }
instead of aPeerInfo
instance(s).BREAKING CHANGE:
refs
andrefs.local
now return an async iterable.BREAKING CHANGE:
object.data
now returns an async iterable that yieldsBuffer
objects.BREAKING CHANGE:
ping
now returns an async iterable.BREAKING CHANGE:
repo.gc
now returns an async iterable.BREAKING CHANGE:
swarm.peers
now returns an array of objects with apeer
property that is aCID
, instead of aPeerId
instance.BREAKING CHANGE:
swarm.addrs
now returns an array of objects{ id: CID, addrs: Multiaddr[] }
instead ofPeerInfo
instances.BREAKING CHANGE:
block.stat
result now contains acid
property (whose value is a CID instance) instead of akey
property.BREAKING CHANGE:
bitswap.wantlist
now returns an array of CID instances.BREAKING CHANGE:
bitswap.stat
result has changed -wantlist
andpeers
values are now an array of CID instances.