fix: pass config to init #303
Conversation
daviddias
added
status/deferred
| const args = ['init'] | ||
| const args = [ | ||
| 'init', | ||
| this.opts.type === 'js' && JSON.stringify(this.opts.config) |
There was a problem hiding this comment.
Can we haz this for Go as well?
There was a problem hiding this comment.
i talked about this in the ipfs pr ipfs/js-ipfs#1662 (comment)
There was a problem hiding this comment.
@Stebalien can you check this ^^
There was a problem hiding this comment.
Go actually takes in an initial config as a file argument (positional). However, it won't merge it. Really, I'm not sure how I'd expect merging to work.
We also have a concept of "profiles" for patching our config.
There was a problem hiding this comment.
@Stebalien we added support for passing a json string has a positional arg to the init cmd like this https://github.com/ipfs/js-ipfs/pull/1662/files
We already had a config options https://github.com/ipfs/js-ipfs#optionsconfig so it was straight forward.
It's just a simple deep merge with the default config.
any chance we can get something similar in go ? we can consider other ways to do it!
we just want to reduce the number of child processes that ctl spawns
Right now we need to spawn 4 child_processes to start a node in ctl
ipfs init
ipfs config show
ipfs config replace
ipfs daemon
With this PR we would only need 2
ipfs init '{"Addresses":{"Swarm":["/ip4/0.0.0.0/tcp/4003"]}}'
ipfs daemon
Final goal is to just do
ipfs daemon --init --config "{}"
This will help a lot making our tests faster!!
There was a problem hiding this comment.
A similar issue has come up with in cluster: ipfs/kubo#6262 (comment) (maybe, I may be misunderstanding the issue).
However, I believe they'll need to pass by-file. Would that work here or will we need two options.
daviddias
force-pushed
the
perf/pass-config-init
branch
from
9c8997c
to
3982cd1
Compare
ghost
added
status/in-progress
|
I've rebased master onto this branch. Tests are passing with updated deps. |
daviddias
added
exp/novice
| const args = ['init'] | ||
| const args = [ | ||
| 'init', | ||
| this.opts.type === 'js' && JSON.stringify(this.opts.config) |
There was a problem hiding this comment.
Can we haz this for Go as well?
This PR aligns `ipfs init` and `ipfs daemon` with go-ipfs. ipfs/kubo#6489 `ipfs init` changed to accept a file path as an argument `ipfs daemon` changed to support `--init` and `--init-config` options. Now we can do `ipfs daemon --init --init-config /path/to/custom-config` refs: ipfs/js-ipfsd-ctl#303
This PR aligns `ipfs init` and `ipfs daemon` with go-ipfs. ipfs/kubo#6489 `ipfs init` changed to accept a file path as an argument `ipfs daemon` changed to support `--init` and `--init-config` options. Now we can do `ipfs daemon --init --init-config /path/to/custom-config` refs: ipfs/js-ipfsd-ctl#303
This PR aligns `ipfs init` and `ipfs daemon` with go-ipfs. ipfs/kubo#6489 `ipfs init` changed to accept a file path as an argument `ipfs daemon` changed to support `--init` and `--init-config` options. Now we can do `ipfs daemon --init --init-config /path/to/custom-config` refs: ipfs/js-ipfsd-ctl#303
|
@hugomrdias what's missing to land? |
|
this change is gonna land with this #369 PR |
This PR aligns `ipfs init` and `ipfs daemon` with go-ipfs. ipfs/kubo#6489 `ipfs init` changed to accept a file path as an argument `ipfs daemon` changed to support `--init` and `--init-config` options. Now we can do `ipfs daemon --init --init-config /path/to/custom-config` refs: ipfs/js-ipfsd-ctl#303
This PR aligns `ipfs init` and `ipfs daemon` with go-ipfs. ipfs/kubo#6489 `ipfs init` changed to accept a file path as an argument `ipfs daemon` changed to support `--init` and `--init-config` options. Now we can do `ipfs daemon --init --init-config /path/to/custom-config` refs: ipfs/js-ipfsd-ctl#303
This PR improves ctl daemon performance passing node config directly to init command.
Needs: