andrewfraley
commented
2 years ago Hi @jbm9, thank you for this. This is really just an oversight and I'm going to address this, but I don't want to remove the ENV variables from the Dockerfile since this makes it much easier to configure when running with tools like Portainer or the Synology Docker manager (the environment variables automatically show up in the GUIs to be modified).
I would welcome your opinion on the following options:
- 1: If any config.ini options have been modified, use the config.ini value unless the ENV var has also been modified
- 2: Use a config.ini value to specify use of config.ini exclusively
- 3: Use an ENV variable to specify use of config.ini exclusively
- 4: Move config.ini to config.ini.example, if config.ini doesn't exist use ENV, if it does exist then use it exclusively.
I think I'm leaning towards option 4.
Thanks!
The ENV settings in the Dockerfile override the config.ini file. Combined with the unit test, this means that it is literally impossible to configure this software without generating diffs to the repository, which is an unfortunate antipattern.
Having environment variables override configuration options is a great way to enable admins to pass secrets into the container without putting them into their config.ini, but having the default Dockerfile override everything is a bit much.
It took me a surprising amount of time to figure out why my config options weren't being applied. Removing the ENV settings enabled me to use config.ini to actually configure things, but resulted in a failing unit test. This also removes the unit test that blindly asserts that those ENV files must exist.
Overall, this is a small change that should be a no-op to anyone running a configured stack, and will enable them to update their local repo without conflicts in Dockerfile.