feat: support adding async iterators #2379

achingbrain · 2019-08-21T17:05:00Z

Adds a ipfs._addAsyncIterator method for adding async iterators and refactors all add methods to call this, as when the Great Async Iterator Migration is complete this will become the one true method to add files to IPFS.

Purposefully does not add symmetrical ipfs._catAsyncIterator methods as this PR is big enough already.

Also fixes up the jsipfs add progress indicator so it works more like the go-ipfs one and prints out hashes as it goes instead of buffering them all up in one go, at the cost of not sorting the outout.

Also, also allows you to control preload behaviour from the command line.

Depends on:

achingbrain · 2019-08-21T17:07:09Z

src/core/components/files-regular/add-async-iterator.js

+      const shouldPin = pin && isRootDir && !opts.onlyHash && !opts.hashAlg
+
+      if (shouldPin) {
+        await ipfs.pin.add(file.hash, { preload: false })


We could probably do this asynchronously as well?

Probably needs to be done in the gc read lock for add though

src/core/components/files-regular/add-async-iterator.js

src/cli/commands/add.js

achingbrain · 2019-08-21T17:11:58Z

src/core/components/files-regular/add-async-iterator.js

+        yield {
+          path: file.path === undefined ? b58Hash : (file.path || ''),
+          hash: b58Hash,
+          size: file.unixfs.fileSize()


This will be different to when you don't specify onlyHash as the node has been serialized so includes the protobuf overhead. Typically we only show the user the hash when they've said onlyHash so I don't know how much of a problem this is. I think we want to switch this over to only show file sizes eventually anyway so it might not be a big deal.

package.json

alanshaw · 2019-08-21T19:04:46Z

src/core/components/files-regular/add-readable-stream.js


-    const retStream = new AddHelper(s, p)
+    const output = new AddHelper(stream, {


Can it-to-stream or stream-to-it help here to avoid buffering and respect backpressure?

Good shout, I'll pull those in.

alanshaw · 2019-08-22T08:16:20Z

src/core/components/files-regular/add-async-iterator.js

+    const iterator = pipe(
+      source,
+      validateInput(),
+      normalizeInput(opts),


I made a comprehensive normalise in https://github.com/ipfs/js-ipfs-http-client/pull/1063/files#diff-7cec0363d7e0ecfb1f2d5e04b55e2cac and https://github.com/ipfs/js-ipfs-http-client/pull/1063/files#diff-8913549ebf1a95e3151ad16513fa9a6f - it would be good to check this is supporting the same inputs (or come to an agreement on what inputs we support).

Nice, maybe we could move that into ipfs-utils?

Yes, will do 👍

I'm going to have a go at moving those utility functions over to ipfs-utils so they can be shared between the http client & core as it's going to simplify this a little.

I'm going to refactor some of it though, because it's full of Buffer.from which involves a buffer copy which is slow.

achingbrain · 2019-08-22T08:42:37Z

I might actually break this up into three PRs - one for ipfs.addAsyncIterator, one for parameterising preload and one for fixing the progress bar. Should make it easier to digest.

achingbrain · 2019-08-22T10:50:33Z

See #2384 and #2386. I'll rebase this once those are in.

Pulled out of #2379. BREAKING CHANGE: Order of output from the CLI command `ipfs add` may change between invocations given the same files since it is not longer buffered and sorted. Previously, js-ipfs buffered all hashes of added files and sorted them before outputting to the terminal, this might be because the `glob` module does not return stable results. This gives a poor user experience as you see nothing then everything, compared to `go-ipfs` which prints out the hashes as they become available. The tradeoff is the order might be slightly different between invocations, though the hashes are always the same as we sort DAGNode links before serializing so it doesn't matter what order you import them in.

alanshaw · 2019-08-27T08:56:35Z

@achingbrain those PRs are now merged 😇

alanshaw

Can we remove glob from dependencies?

alanshaw

LGTM, pending dependency updates and CI passing

achingbrain · 2019-08-29T14:57:50Z

Can we remove glob from dependencies?

Not yet - it's still used in src/cli/commands/commands.js and src/core/components/init-assets.js.

Some ipfs.add() tests need to be skipped until ipfs#2379 is merged License: MIT Signed-off-by: Marcin Rataj <lidel@lidel.org>

achingbrain · 2019-09-07T14:01:26Z

There are a few addFromUrl interface tests new in v0.113.0 failing on Windows but they will be fixed by ipfs/js-datastore-fs#27

Adds a `ipfs._addAsyncIterator` method for adding async iterators and refactors all add methods to call this, as when the Great Async Iteator Migration is complete this will become the one, true method to add files to IPFS.

achingbrain · 2019-09-08T08:19:25Z

This switch to js-ipfs before PR-2379 ipfs/js-ipfs#2379 switched ipfs.add to ipfs._addAsyncIterator and it broke /api/v0/add exposed by embedded js-ipfs in Brave. It seems old polyfills are no longer enough. Real fix requires more time to investigate, so for now we switch to version before js-ipfs/PR-2379. Used js-ipfs commit is from ipfs/js-ipfs#2304 before it was rebased on top of master after PR-2379, making it the last safe version. Real fix will be tracked in #757

achingbrain added the status/in-progress In progress label Aug 21, 2019

achingbrain commented Aug 21, 2019

View reviewed changes

src/core/components/files-regular/add-async-iterator.js Outdated Show resolved Hide resolved

achingbrain commented Aug 21, 2019

View reviewed changes

src/cli/commands/add.js Show resolved Hide resolved

achingbrain commented Aug 21, 2019

View reviewed changes

src/cli/commands/add.js Outdated Show resolved Hide resolved

achingbrain commented Aug 21, 2019

View reviewed changes

package.json Outdated Show resolved Hide resolved

alanshaw reviewed Aug 21, 2019

View reviewed changes

alanshaw reviewed Aug 22, 2019

View reviewed changes

This was referenced Aug 22, 2019

feat: allow controlling preload behaviour from cli #2384

Merged

fix: make progress bar work again when adding files #2386

Merged

alanshaw mentioned this pull request Aug 27, 2019

⚡️ v0.38.0 RELEASE 🚀 #2396

Closed

66 tasks

achingbrain force-pushed the add-async-iterators branch 2 times, most recently from 0dfd370 to dd4fd43 Compare August 27, 2019 14:49

alanshaw reviewed Aug 29, 2019

View reviewed changes

alanshaw approved these changes Aug 29, 2019

View reviewed changes

achingbrain force-pushed the add-async-iterators branch from dd4fd43 to bd31372 Compare August 29, 2019 15:01

achingbrain mentioned this pull request Aug 31, 2019

feat: add normalise input function ipfs/js-ipfs-utils#5

Merged

alanshaw mentioned this pull request Sep 3, 2019

Update interface-ipfs-core to the latest version 🚀 #2420

Closed

lidel added a commit to lidel/js-ipfs that referenced this pull request Sep 5, 2019

fix(ci): skip tests without implementation

693182f

Some ipfs.add() tests need to be skipped until ipfs#2379 is merged License: MIT Signed-off-by: Marcin Rataj <lidel@lidel.org>

lidel added a commit to lidel/js-ipfs that referenced this pull request Sep 5, 2019

fix(ci): skip tests without implementation

c239abd

Some ipfs.add() tests need to be skipped until ipfs#2379 is merged License: MIT Signed-off-by: Marcin Rataj <lidel@lidel.org>

alanshaw mentioned this pull request Sep 5, 2019

Update ipfs-http-client to the latest version 🚀 #2426

Closed

achingbrain force-pushed the add-async-iterators branch 3 times, most recently from 6d990a7 to 687d8d0 Compare September 6, 2019 16:28

achingbrain force-pushed the add-async-iterators branch 4 times, most recently from 376ae40 to e0f4e9b Compare September 6, 2019 17:59

achingbrain added 2 commits September 7, 2019 15:02

feat: support adding async iterators

3a56e72

Adds a `ipfs._addAsyncIterator` method for adding async iterators and refactors all add methods to call this, as when the Great Async Iteator Migration is complete this will become the one, true method to add files to IPFS.

chore: start echo server before tests

29a541b

achingbrain force-pushed the add-async-iterators branch from 02a25b8 to 29a541b Compare September 7, 2019 14:02

achingbrain merged commit 3878f0f into master Sep 9, 2019

achingbrain deleted the add-async-iterators branch September 9, 2019 09:45

This was referenced Sep 9, 2019

fix: limit concurrent HTTP requests in browser #2304

Merged

Brave: regression in embedded /api/v0/add ipfs/ipfs-companion#757

Closed

alanshaw mentioned this pull request Oct 21, 2019

Awesome Endeavour: Async Iterators #1670

Closed

snyk-bot mentioned this pull request Dec 6, 2019

[Snyk] Upgrade ipfs from 0.37.1 to 0.39.0 dyna-dot/orbit-web#1

Open

github-actions bot mentioned this pull request Sep 22, 2021

chore: release js-ipfs 1.0.0 #3887

Closed

github-actions bot mentioned this pull request Jan 25, 2022

chore: release js-ipfs 0.1.0 #4026

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: support adding async iterators #2379

feat: support adding async iterators #2379

achingbrain commented Aug 21, 2019 •

edited by alanshaw

Loading

achingbrain Aug 21, 2019

alanshaw Aug 22, 2019

achingbrain Aug 21, 2019

alanshaw Aug 21, 2019

achingbrain Aug 22, 2019

alanshaw Aug 22, 2019

achingbrain Aug 22, 2019

alanshaw Aug 23, 2019

achingbrain Aug 30, 2019

achingbrain Aug 30, 2019

achingbrain commented Aug 22, 2019 •

edited

Loading

achingbrain commented Aug 22, 2019

alanshaw commented Aug 27, 2019

alanshaw left a comment

alanshaw left a comment

achingbrain commented Aug 29, 2019

achingbrain commented Sep 7, 2019

achingbrain commented Sep 8, 2019


		const retStream = new AddHelper(s, p)
		const output = new AddHelper(stream, {

feat: support adding async iterators #2379

feat: support adding async iterators #2379

Conversation

achingbrain commented Aug 21, 2019 • edited by alanshaw Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

achingbrain commented Aug 22, 2019 • edited Loading

achingbrain commented Aug 22, 2019

alanshaw commented Aug 27, 2019

alanshaw left a comment

Choose a reason for hiding this comment

alanshaw left a comment

Choose a reason for hiding this comment

achingbrain commented Aug 29, 2019

achingbrain commented Sep 7, 2019

achingbrain commented Sep 8, 2019

achingbrain commented Aug 21, 2019 •

edited by alanshaw

Loading

achingbrain commented Aug 22, 2019 •

edited

Loading