Skip to content
This repository was archived by the owner on Oct 3, 2025. It is now read-only.

Conversation

@farrago
Copy link

@farrago farrago commented Mar 27, 2021

This initially replaces the manual conf.json file loading and parsing with viper. The same file is still loaded from the same location and fills the same configuration struct, so there are no backwards compatibility issues.

Using viper then adds the ability to override config settings with an environment variable in the form "FEATMAP_<uppercase setting name>". e.g. FEATMAP_PORT=8080 would override the port value in conf.json.

Due to the way Viper works, this cannot load from only env vars, so the config file is still needed and the env vars can only override it. See spf13/viper#584

To help protect users from this issue, this change also adds verification that essential config settings have been configured.

Tests

  • Test that configuration is still loaded from the config file
  • Test that environment variables override the values in the config file
  • Test that missing settings correctly report an error
  • Test loading without conf.json and check the correct error is reported

farrago added 3 commits April 17, 2021 12:29
This replaces the manual file loading and parsing with
 [viper](https://github.com/spf13/viper). In this commit,
 the functionality is identical, but using viper will allow
 additional configuration options in the future.

 # Tests

 - Test loading from existing conf.json works and loads correct values
 - Test missing file is correctly reported as an error
 - Test default value for SMTPPort works when it's not in the conf.json
This adds the ability to override config settings with an environment
variable in the form "FEATMAP_<uppercase setting name>".
e.g. `FEATMAP_PORT=8080` would override the `port` value in `conf.json`.

Due to the way Viper works, this cannot load from *only* env vars,
so the config file is still needed and the env vars can override it.
See spf13/viper#584

To help protect users from this issue, this change also adds
verification that essential config settings have been configured.

# Tests

- Test that configuration is still loaded from the config file
- Test that environment variables override the values in the config file
- Test that missing settings correctly report an error
Update to only check if any configuration is missing if conf.json
was loaded.  If it wasn't loaded, leave the failure to load as the reported
error.

# Tests

- Test loading without `conf.json` and check the correct error is reported
- Test loading with `conf.json` but missing some entries and check errors
- Test loading with complete config and check it loads correctly
@amborle
Copy link
Owner

amborle commented Jul 1, 2021

Thanks for submitting the PR!

I really don't like that you need to have a configuration file even though all variables are supplied as envs.

Found this: https://github.com/joho/godotenv. Any obvious downsides going with this instead?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants