-
Notifications
You must be signed in to change notification settings - Fork 3
Conversation
Thanks for opening this pull request! The maintainers of this repository would appreciate it if you would create a changelog item based on your changes. |
} else { | ||
err := godotenv.Load(path.Join(v, defaultFilename+".env")) | ||
if err != nil { | ||
log.Debug().Msgf("ignoring missing env file on dir: %v", v) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
log.Debug().Str("dir", v).Msg("ignoring missing env file")
:P
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@tboerger Nice to see you!
log := NewLogger(config.New()) | ||
for _, v := range defaultConfigPaths { | ||
// location is the user's home | ||
if v[0] == '$' || v[0] == '~' { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't you just replace something like $HOME
or ~
with usr
? :)
Do I understand correctly that this adds another mechanism to configure an extension by reading the .env files from multiple locations? If we are aiming at https://12factor.net/config can we get rid of toml, json or yaml style config files and stick to one way of configuring the extension? |
No, this still uses Viper to load config values, it doesn't add a new mechanism per se. The fact that Viper doesn't merge environment variables, and that it is a bug that is hard for them to change, makes clients (of Viper) do this workarounds. It really doesn't bring anything new to the mix, as the file name conventions still apply, and file location still apply. That said, the way it works is by reading in any of the configured locations for, in this case, an You could, as you said, set the config type to p.s: to address your concern, yes the library has an option to read from multiple sources, but it's not in use here. |
Nothing changed, comments are still ignored 🤯 |
At least we need to react. |
Goal
Closes owncloud/product#16
We want to be able to load config values from dotenv files.
Usage
No changes from the original functionality. To get this up and running the only thing you need is to write your
extension.env
file (i.e: accounts.env, file name is conventional) and run your extension.For local testing I recommend the following setup:
cd into your ocis extensions location and run:
go run cmd/ocis-{extension_name}/main.go server
You should immediately see the logs with the newly loaded values.
How
While Viper has native support for it, it does not merge environment variables onto structs for a whole set of reasons that you can read here.
The way it works is by loading the environment variables on the
.env
file onto your actual environment, ready to be picked up by Viper.The library also offers its own parser, which means we can have comments on such files, like so:
Precedence
extract from godotenv's Readme
If you need to, you can also use godotenv.Overload() to defy this convention and overwrite existing envs instead of only supplanting them. Use with caution.
This DOES NOT modify the way an extension is configured
P.S: Props to @tboerger for pointing this wonderful library out 🙌