Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: support pull streams #8

Merged
merged 2 commits into from
Sep 3, 2019
Merged

Conversation

alanshaw
Copy link
Member

@alanshaw alanshaw commented Sep 3, 2019

This PR updates the normaliseInput function to accept pull streams.

I've also made the following changes:

  1. Update the docs for supported inputs
    • Buffer|ArrayBuffer|TypedArray is aliased as Bytes
    • Blob|File is aliased as Bloby
    • Added info for what a input "means" i.e. causes single/multiple files to be added
  2. Peek the first item of an (async) iterator properly
  3. Move file object check below input[Symbol.asyncIterator] check because Node.js streams have a path property that will false positive the isFileObject check
  4. Fix toFileObject to allow objects with no content property
  5. Simplify toBuffer to remove checks that Buffer.from already does

This PR updates the `normaliseInput` function to accept pull streams.

I've also made the following changes:

1. Update the docs for supported inputs
  * `Buffer|ArrayBuffer|TypedArray` is aliased as `Bytes`
  * `Blob|File` is aliased as `Bloby`
  * Added info for what a input "means" i.e. causes single/multiple files to be added
1. Peek the first item of an (async) iterator properly
1. Move file object check below `input[Symbol.asyncIterator]` check because Node.js streams have a path property that will false positive the `isFileObject` check
1. Fix `toFileObject` to allow objects with no `content` property
1. Simplify `toBuffer` to remove checks that `Buffer.from` already does

License: MIT
Signed-off-by: Alan Shaw <alan.shaw@protocol.ai>
@alanshaw alanshaw requested a review from achingbrain September 3, 2019 13:04
License: MIT
Signed-off-by: Alan Shaw <alan.shaw@protocol.ai>
@achingbrain
Copy link
Member

If these inputs do actually result in multiple files (e.g. the collections have more than one entry):

Iterable<Bloby> [multiple files]
Iterable<String> [multiple files]
AsyncIterable<Bloby> [multiple files]
AsyncIterable<String> [multiple files]
PullStream<Bloby> [multiple files]
PullStream<String> [multiple files]

and they aren't added with the wrapWithDirectory option they'll cause the importer to return an error as they will result in multiple roots.

I guess we just let it fail there? Otherwise we have to validate the options too.

@alanshaw
Copy link
Member Author

alanshaw commented Sep 3, 2019

IIRC you can add multiple files as long as they're all in the same root...so we need these inputs for that at least. Multiple roots is interesting because I think that go-ipfs supports this and I know that we don't. I'm ok for the importer to error in this case until it can be fixed...

@achingbrain achingbrain merged commit e5b2509 into normalise-input Sep 3, 2019
@achingbrain achingbrain deleted the norm-in-pullstream branch September 3, 2019 15:32
hugomrdias pushed a commit that referenced this pull request Sep 4, 2019
* feat: consolidate ipfs.add input normalisation

Allows input normalisation function to be shared between ipfs and the
http client.

* feat: support pull streams (#8)

* feat: support pull streams

This PR updates the `normaliseInput` function to accept pull streams.

I've also made the following changes:

1. Update the docs for supported inputs
  * `Buffer|ArrayBuffer|TypedArray` is aliased as `Bytes`
  * `Blob|File` is aliased as `Bloby`
  * Added info for what a input "means" i.e. causes single/multiple files to be added
1. Peek the first item of an (async) iterator properly
1. Move file object check below `input[Symbol.asyncIterator]` check because Node.js streams have a path property that will false positive the `isFileObject` check
1. Fix `toFileObject` to allow objects with no `content` property
1. Simplify `toBuffer` to remove checks that `Buffer.from` already does
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants