Skip to content
This repository has been archived by the owner on Mar 10, 2020. It is now read-only.

API proposal for ipfs.files.add #20

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
93 changes: 92 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ interface-ipfs-core API.

![](/img/badge.png)

# How to use the battery tests
# How to use the battery of tests

## Node.js

Expand Down Expand Up @@ -55,6 +55,97 @@ test.all(common)

A valid (read: that follows this interface) IPFS core implementation, must expose the following API.

## Files

### `add`

> Add files and data to IPFS.

##### `Go` **WIP**

##### `JavaScript` - ipfs.files.add(data, [callback])

Where `data` may be

- an array of objects, each of the form
```js
{
path: '/tmp/myfile.txt',
content: (Buffer or Readable stream)
}
```
- a `Buffer` instance
- a `Readable` stream

`callback` must follow `function (err, res) {}` signature, where `err` is an
error if the operation was not successful. `res` will be an array of

```js
{
path: '/tmp/myfile.txt',
node: DAGNode
}
```

If no `callback` is passed, a promise is returned.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What does the Promise resolve to? An array of DAGNodes?

We should add examples for the returns

ipfs.files.add(path).then((res) => console.log(res)) // --> [DAGNode1, DAGNode2, ...] ???


Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add: example:

Example:
```js
var files = [
{
path: '/tmp/myfile.txt',
content: (Buffer or Readable stream)
}
]
ipfs.files.add(files, function (err, files) {
// 'files' will be an array of objects
})
```


### `createAddStream`

> Add files and data to IPFS using a transform stream.

##### `Go` **WIP**

##### `JavaScript` - ipfs.files.createAddStream([callback])

Provides a Transform stream, where objects can be written of the forms
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Provides a Duplex stream

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I spent some time thinking about this, and I'm convinced that it's actually a Transform stream. Here is the article that helped me draw a clearer line of differentiation between the two.

It's a Transform because the input and the output are directly related: each object written is outputted as an object that's been transformed (/w side effects) through the IPFS API.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are more objects emitted than those who are written. E.g

/a/b/c.txt

will emit 3 times, one for the file and 2 other for the dirs.

I understand your reasoning, but feels weird to call it a Transform, it would be like calling a TCP socket a Transform stream, since we will read mutated output. I think both will work

@dignifiedquire @haadcode thoughts?

Copy link
Contributor Author

@hackergrrl hackergrrl May 26, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Transform streams actually very commonly emit more or fewer elements than they are given: a zlib compress transform will yield fewer or more bytes depending on direction; a binary fsk encoder (https://github.com/noffle/binary-fsk) will accept byte buffer objects but emit way more audio data depending on the sample rate.

The key distinction is that in Transform streams, the output is in some way calculated from the input. This is in contrast to a Duplex stream (e.g. TCP), which is much more akin to two people on a telephone exchanging messages.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A transform is stream is just a specialised version of a duplex stream, so really both are correct.


```js
{
path: '/tmp/myfile.txt',
content: (Buffer or Readable stream)
}
```
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is related with having it returning a full DAGNode (#20 (comment)), which from what I read you also agree, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I would like DAGNodes returned, but doesn't it come at a high cost? I.e. Won't doing an extra object.get call per added node mean N extra API requests against Core (in js-ipfs-api land, N HTTP requests)? Or is there some sort of batching object.get mechanism I don't know about?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The 'high cost' is virtual, because currently it is a given fact that the http-api needs improvements and it is better to provide a richer and consistent interface at a cost of another RTT, feeding back that into the http-api design, that simply cut it off.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Revision made.


`callback` must follow `function (err, stream) {}` signature, where `err` is an
error if the operation was not successful. `stream` will be a Transform stream,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Duplex stream

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

see above

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

to which tuples like the above two object formats can be written and [DAGNode][]
objects will be outputted.

If no `callback` is passed, a promise is returned.

```js
ipfs.files.createAddStream(function (err, stream) {
stream.on('data', function (file) {
// 'file' will be of the form
// {
// path: '/tmp/myfile.txt',
// node: DAGNode
// }
})

stream.write({path: <path to file>, content: <buffer or readable stream>})
// write as many as you want

stream.end()
})
```

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If no data is passed, then, a duplex stream is returned, that pretty much is the duplex stream returned by unixfs-engine Importer

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What if no data is passed and no callback is passed? The possible return matrix starts to get messy -- you'd want a stream and a promise back. ;)

Copy link
Contributor

@daviddias daviddias May 21, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That would be a misuse of the API.

Note that we need the ability to return a duplex stream to be able to 'add files as they come', through the http-api or through the cli.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can have the callback/promise provide a stream if no data is passed, but we can't have the function actually return a stream ever, since we went down the road of always returning promises.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's return the duplex stream on the callback on the interface-ipfs-core. This does not require changes on unixfs-engine. Sounds good?



## Object

### `object.new`
Expand Down
5 changes: 4 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -28,9 +28,12 @@
},
"homepage": "https://github.com/ipfs/interface-ipfs-core#readme",
"dependencies": {
"bl": "^1.1.2",
"bs58": "^3.0.0",
"chai": "^3.5.0",
"ipfs-merkle-dag": "^0.6.0"
"detect-node": "^2.0.3",
"ipfs-merkle-dag": "^0.6.0",
"readable-stream": "1.1.13"
},
"devDependencies": {
"aegir": "^3.0.4"
Expand Down
Binary file added test/data/15mb.random
Binary file not shown.
Loading