Open jbenet opened 9 years ago
What's the order of precedence?
CLI arg > ENV > config?
i think i like @maybebtc's order of precedence.
that order of precedence seems right. Let's go with that until we find a strong case to deviate.
ENV vars should be removed.
They are not tied to a specific daemon, hence not loggable (also who knows who/what set them and if they affect child processes). ENV vars for initial bootstrap are fine, but if further enabled they could be abused for persistent config in a bashrc. Also, extra overhead because people have to remember config precedence.
Besides ipfs config
already provides the same convenience.
Reminds me of a mess in geant4 (a particle simulator framework) installation before they switched to cmake nowadays (https://aur.archlinux.org/packages/ge/geant4/PKGBUILD, https://github.com/Homebrew/homebrew-science/blob/master/geant.rb).
ENV vars are used very often for all sorts of things. unfortunately it's the easiest way to setup deployments in many kinds of deployment systems (things like heroku and so on). I dont think we should remove them.
They are not tied to a specific daemon, hence not loggable (also who knows who/what set them and if they affect child processes). ENV vars for initial bootstrap are fine, but if further enabled they could be abused for persistent config in a bashrc. Also, extra overhead because people have to remember config precedence.
Agreed with all this, but there are cases where it makes more sense to use ENV vars, or mutating a file on disk is not a simple endeavor, and so using env vars as part of a command that gets run is the easiest thing. (it's not feasible to put everything as cli args).
ssh user@host IPFS_PATH=/foo/bar/ipfs ipfs daemon --init
ssh user@host IPFS_PATH=/foo/bar/ipfs ipfs pin add -r <path>
Issue related -> https://github.com/ipfs/go-ipfs/issues/251
If #6325 is merged this issue can be closed as we will have decided on a direction.
That PR makes it possible to configure a go-ipfs container using env variables but doesn't apply to go-ipfs itself. Personally, I'd kind of like to use environment variables for debugging and only debugging (except for containers). Current list: https://github.com/ipfs/go-ipfs/blob/master/docs/environment-variables.md
Why not use initialization scripts for containers? I think it is a common practice for database containers:
https://hub.docker.com/_/postgres
If you would like to do additional initialization in an image derived from this one, add one or more .sql, .sql.gz, or .sh scripts under /docker-entrypoint-initdb.d (creating the directory if necessary). After the entrypoint calls initdb to create the default postgres user and database, it will run any .sql files, run any executable .sh scripts, and source any non-executable .sh scripts found in that directory to do further initialization before starting the service.
Related clarification from https://github.com/ipfs/go-ipfs/pull/8326#issuecomment-912726852:
If we ever add
ipfs config
override via env variable, it should be supported in go-ipfs natively (go-ipfs/docs/environment-variables.md
), and work as "ad-hoc" ephemeral override that does not mutate the config file.
We have three ways to express configuration variables to
ipfs
:$IPFS_DIR/config
fileThis is both good and bad. Good because all of these methods have specific use cases that make them much more preferrable to others (e.g. env vars in deployments, cli options are faster to change while experimenting than a config file, etc). Bad because it can create lots of confusion as to where each of these options should be defined in the code, and which methods it should support.
Here are some thoughts. Not sure what the right trajectory is yet.
Config
exclusively (we have no access to options, AND should never useos.GetEnv
). This means that all config variables the node can choose from have to be set on the config. (i think so far we've adhered to this).cmd/env.go
, read from the env, and set them on map on thecmdInvocation
, or the execution context. Then, we could either force Config overrides to happen (i.e. env vars can only override config values) or Commands may access the env variables from the execution context. BUT, noteos.GetEnv
is disallowed (forcing users of env vars to add them to the one place we define them all, so we have them all listed in one file).os.GetEnv("NO_FUSE")
from tests.