WIP: Feat/toggle ipfs implementation #726
Conversation
|
This is looking good!
Good point. I just checked and js-ipfs doesn't support those flags yet. We'll need something more like a deamon creator strategy that encapsulates the type of node we want to start and the args that it should be run with.
Desktop will start with go-ipfs as the default option, so the repo will have been upgraded. We just need to test that the version of js-ipfs we bundle works with the same repo version as the go-ipfs version that we bundle. That's true for the latest releases of both right now, so we're good. We just need to keep an eye out for future releases that update the repo version, and wait till both js and go are in sync again before releasing an update. as the routing option is just an optimisation, it's ok to leave it out when using js-ipfs. The upcoming 0.34 release is adding more DHT support to js-ipfs, so that may change soon too.
Yep that's fine. we can bundle both into the release so the user can pick between the two. It'll make the app larger, but I'm ok with that. If we want to get fancy in the future we can lazily fetch js-ipfs if the user chooses it. We could even allow picking explicit version numbers to allow comarisions and benchmarking, but it's not neeeded right now.
I think a select box is the right control for the question were asking the user. We're offering the choice between go-ipfs or js-ipfs rather than a "do you want js-ipfs on or off?"
No need to squash commits in the PR, I'll do that when we merge to master. You'll need to merge changes in from master...again, it's fine to merge, as I'll squash it all later into a single commit on master. The package-lock.json is a feature of npm to let app developers pin all the transitive dependencies in the the node_modules, and share them amoungst the team. Adding js-ipfs to the package.json will update the package-lock.json too, so that's fine! shout if you have any more questions. We can jump on a call if that'd help. We also have a weekly team catch up call on zoom every Wedneday at 4pm UTC if you'd like to join! You can see it on the IPFS communuity calendar https://calendar.google.com/calendar/embed?src=ipfs.io_eal36ugu5e75s207gfjcu0ae84@group.calendar.google.com&ctz=UTC |
|
Closing (stale). Please re-open if needed. Thanks! |
Seeking feedback. Working on enabling toggling between IPFS implementations.
I'm running into an issue. Essentially, after experimenting with a select component for the toggle in the WebUI settings page, but realized that it would add too much code for something that is essentially just a toggle. So, instead of a select, I went with the checkbox which allowed me to just reuse most of the existing logic and just add a new hook. Now the code I wrote (in src/hooks/implementation.js) executes on startup - it doesn't seem to have 'go' as a default, and I'm having trouble sorting out how to prevent it from trying to start the daemon a second time. At least I think that's what this error means:
(node:26947) UnhandledPromiseRejectionWarning: Error: connect ECONNREFUSED 127.0.0.1:5001 at TCPConnectWrap.afterConnect [as oncomplete] (net.js:1161:14) (node:26947)Here are a few more things that I know I'm seeking feedback on, but of course I'm open to any feedback.
What should I do with the flags in the daemon creator? I found that I needed to disable them to get the JS node to spawn, but I'm not sure what they do. Obviously, I'll want to re-enable those flags whenever go is enabled for the final commit, but what should I do when js is enabled?
Re: dependencies: I've got both ipfs and js-ipfs added as dependencies. Are both necessary or are they redundant? If so, should i pick one over the other?
Not in this PR but as far as the webui: is a checkbox sufficient for this or is the select preferable?
Obviously need to do some housekeeping: squash commits, rollback the change of apiAddress in webui/index.js , remove the logger in src/utils/daemon.js. What other similar tasks jump out to you right away? I'm unfamiliar with package-lock files, and not sure what I did to change this one, but is this something that should go in my git-ignore? globally?