Skip to content
This repository has been archived by the owner on Feb 12, 2024. It is now read-only.

[WIP] feat: upgrade the Files API #1671

Closed
wants to merge 8 commits into from
Closed

[WIP] feat: upgrade the Files API #1671

wants to merge 8 commits into from

Conversation

daviddias
Copy link
Member

@ghost ghost assigned daviddias Oct 26, 2018
@ghost ghost added the status/in-progress In progress label Oct 26, 2018
@daviddias
Copy link
Member Author

Currently blocked by tests on master failing all together

@daviddias daviddias changed the title Upgrade the Files API feat: upgrade the Files API Nov 3, 2018
@daviddias daviddias changed the title feat: upgrade the Files API [WIP] feat: upgrade the Files API Nov 3, 2018
@daviddias
Copy link
Member Author

daviddias commented Nov 4, 2018

Started seeing these now:

(node:75777) UnhandledPromiseRejectionWarning: AssertionError: expected [Error: Please run jsipfs init first] to not exist
    at factory.spawn (/Users/imp/code/js-ipfs/test/core/files.spec.js:26:26)
    at once (/Users/imp/code/js-ipfs/node_modules/ipfsd-ctl/src/factory-in-proc.js:114:16)
    at f (/Users/imp/code/js-ipfs/node_modules/once/once.js:25:25)

Investigating, might be related with recent js-ipfsd-ctl release

Nope. it is really something I'm doing here.

@@ -3,38 +3,37 @@
const promisify = require('promisify-es6')
const mfs = require('ipfs-mfs/core')

module.exports = self => {
const mfsSelf = Object.assign({}, self)
Copy link
Member Author

Choose a reason for hiding this comment

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

@achingbrain I'm not entirely certain of all the consequences of doing this (given all the internal state that is kept by so many pieces. It doesn't look like a good practice or something that we want to debug later. If you know this is super safe, please do enlighten me as I would love to understand how.

On second note, what we really want is to avoid circular dependencies. js-ipfs-mfs should only take from IPFS what it needs and not the whole object.

return mfs(mfsSelf, mfsSelf._options)
module.exports = (self) => {
const proxy = {
add: self.add,
Copy link
Member Author

Choose a reason for hiding this comment

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

this is the only call I saw MFS using other than dag.get and dag.put

@daviddias
Copy link
Member Author

MFS is not happy when it doesn't have the full IPFS object


  files
Swarm listening on /ip4/127.0.0.1/tcp/50095/ipfs/QmNk8vGmncjQfZHyCvk1AQkFukKjgmHfKehYi8BL3fC5F5
(node:85366) UnhandledPromiseRejectionWarning: AssertionError: expected [Error: Please run jsipfs init fi
    at factory.spawn (/Users/imp/code/js-ipfs/test/core/files.spec.js:26:26)
    at once (/Users/imp/code/js-ipfs/node_modules/ipfsd-ctl/src/factory-in-proc.js:114:16)
    at f (/Users/imp/code/js-ipfs/node_modules/once/once.js:25:25)
    at /Users/imp/code/js-ipfs/node_modules/async/internal/parallel.js:39:9
    at /Users/imp/code/js-ipfs/node_modules/async/internal/once.js:12:16
    at iterateeCallback (/Users/imp/code/js-ipfs/node_modules/async/internal/eachOfLimit.js:45:17)
    at /Users/imp/code/js-ipfs/node_modules/async/internal/onlyOnce.js:12:16
    at /Users/imp/code/js-ipfs/node_modules/async/internal/parallel.js:36:13
    at exec.start (/Users/imp/code/js-ipfs/node_modules/ipfsd-ctl/src/ipfsd-in-proc.js:201:16)
    at done (/Users/imp/code/js-ipfs/src/core/components/start.js:13:16)
    at /Users/imp/code/js-ipfs/node_modules/async/internal/parallel.js:39:9
    at /Users/imp/code/js-ipfs/node_modules/async/internal/once.js:12:16
    at iterateeCallback (/Users/imp/code/js-ipfs/node_modules/async/internal/eachOfLimit.js:45:17)
    at /Users/imp/code/js-ipfs/node_modules/async/internal/onlyOnce.js:12:16
    at /Users/imp/code/js-ipfs/node_modules/async/internal/parallel.js:36:13
    at self.files.stat (/Users/imp/code/js-ipfs/src/core/mfs-preload.js:39:25)
    at mutex.(anonymous function).then.catch (/Users/imp/code/js-ipfs-mfs/src/core/utils/create-lock.js:4
    at process._tickCallback (internal/process/next_tick.js:68:7)

Will continue to investigate

@daviddias
Copy link
Member Author

Note to self

image

@daviddias
Copy link
Member Author

Is there anyone available to take this to the finish line? I'm afraid I won't be able to do it for another 1,5/2 weeks

@daviddias
Copy link
Member Author

With ipfs-inactive/interface-js-ipfs-core#378 merged, this PR and respective release becomes P0 - Critical as now the documentation says something different from what js-ipfs does.

@daviddias daviddias added the P0 Critical: Tackled by core team ASAP label Nov 17, 2018
@alanshaw
Copy link
Member

@daviddias - on it! I'll get this to a state where it can be merged. I need to add tests to interface-ipfs-core for #1552 so I'll need this merged before then.

I'll temporarily skip the addFrom* tests here while I finish off #1552 and circle back to implementing addFrom* as a separate PR after.

License: MIT
Signed-off-by: Alan Shaw <alan.shaw@protocol.ai>
@alanshaw alanshaw mentioned this pull request Nov 19, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
P0 Critical: Tackled by core team ASAP
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants