Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow setting the config path through an environment variable #6444

Merged
merged 1 commit into from
Apr 22, 2016

Conversation

jsternberg
Copy link
Contributor

The config path previously could only be specified through the command
line options. This made it very difficult to set a default config path
without using any option.

Now the environment variable can be set so the default configuration
path is set to a specific place, but can be overwritten using the
command line option.

The primary purpose of this is so the Docker container can have a
default configuration file, but not have to parse the command line
options to figure out if a different configuration file has been
specified while still allowing the user to only type influxd and have
the program start correctly.

This might also help #6392 as it would allow a default configuration
location to be included with the package by setting an environment
variable.

@jsternberg
Copy link
Contributor Author

@jwilder @beckettsean

This change was primarily needed for the Docker image in the future, but I thought it might also be relevant to #6392.

@jwilder
Copy link
Contributor

jwilder commented Apr 21, 2016

Might be nice to have a default config search path. e.g if no -config is specified, look for ~/.influxdb/influxdb.con, then '/etc/influxdb/influxdb.conf and fall back to default config if none exist.

The docker image wouldn't need to specifiy the -config flag if you mount the config into /etc/influxdb with the correct name then.

Env var as you have in the PR is still useful though. So 👍 on green.

@jwilder
Copy link
Contributor

jwilder commented Apr 21, 2016

Needs changelog under features though.

@jsternberg
Copy link
Contributor Author

@e-dard this is useful for the Docker images. We won't be able to include it in the 0.12 images, but it should make the entrypoint script more simple for 0.13.

@jsternberg
Copy link
Contributor Author

@jwilder I'll add that in. What should be the order that's checked?

  1. CLI option
  2. Environment variable
  3. Default search path

@e-dard
Copy link
Contributor

e-dard commented Apr 21, 2016

LGTM 👍

@jwilder
Copy link
Contributor

jwilder commented Apr 21, 2016

@jsternberg 3, 2, 1. More specific options overrides. The current config options work by using the config file, env var and finally CLI flags override those. Should work consistently with the existing model.

@jsternberg jsternberg force-pushed the js-config-from-env-var branch 3 times, most recently from efb30bd to c0e078e Compare April 22, 2016 13:27
The config path previously could only be specified through the command
line options. This made it very difficult to set a default config path
without using any option.

Now the environment variable can be set so the default configuration
path is set to a specific place, but can be overwritten using the
command line option.

The primary purpose of this is so the Docker container can have a
default configuration file, but not have to parse the command line
options to figure out if a different configuration file has been
specified while still allowing the user to only type `influxd` and have
the program start correctly.

This might also help #6392 as it would allow a default configuration
location to be included with the package by setting an environment
variable.

A default search path is also provided now with checking the following
paths for a config file when none is specified:

* `~/.influxdb/influxdb.conf`
* `/etc/influxdb/influxdb.conf`

The config command has also been modified to read this config file
before outputting a sample config.
@jwilder jwilder added this to the 0.13.0 milestone Apr 22, 2016
@jwilder
Copy link
Contributor

jwilder commented Apr 22, 2016

👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants