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

Split up the API by functions/purpose #886

Open
sagikazarmark opened this issue Apr 9, 2020 · 0 comments
Open

Split up the API by functions/purpose #886

sagikazarmark opened this issue Apr 9, 2020 · 0 comments

Comments

@sagikazarmark
Copy link
Collaborator

Currently Viper's API is implemented in a single, facade-like object and can be grouped six five categories:

Configuration API

The behavior of Viper can be configured through a number of setters:

  • SetConfigFile
  • SetConfigName
  • SetConfigPermissions
  • BindFlagValue
  • AutomaticEnv
  • etc (see the complete, up to date list here)

As of 1.6.0 the key delimiter can also be changed by passing functional options to the NewWithOptions constructor.

Config loading API

Configuration can be loaded from a number of sources, some of those require configuration, some of them require manual loading:

  • ReadInConfig
  • MergeConfig
  • etc

These functions are different from the configuration functions in the sense that they don't (necessarily) modify the behavior of Viper, just load the values from the configured sources, storing them in the Viper instance.

It's worth noting here, that some sources (like flags) require an external actor (FlagSet) to "load" values. (In case of flags, Viper registers itself in the FlagSet as a visitor for flags when one of the bind functions is called)

Config writer API

Viper allows the loaded configuration to be written to the filesystem.

Watcher API

Some sources (file, remote) provide a way to get notified about changes and reload Viper (and the application) with the new values.

Config setter API

Allows setting certain values in Viper:

  • defaults
  • value overrides

Config getter API

This is by far the largest API surface in Viper. It allows fetching values from the loaded and merged configuration in various ways:

  • IsSet: check for a value being set
  • InConfig: check if a value is in the config file
  • Get*: get a single value, casted to the * type
  • Unmarshal: unmarshal configuration onto a struct
  • AllKeys, AllSettings: get all keys and settings
  • Sub: returns a new Viper instance with all of the configuration under a certain key path

While Viper is certainly one of the most popular (if not the most popular) config loading library, and for a huge part it's thanks to the relatively simple, but complete API. You get everything in one place, with almost zero configuration.

Unfortunately, the same API causes pain to many people, because these API groups can hardly be used concurrently:

  • watching and writing at the same time is racy
  • changing config and loading at the same time is racy
  • using Sub with loading and writing is undefined
  • etc

These issues mostly exist because Viper can be used for reading, writing, watching at the same time.

There is also performance hit of being able to read and modify configuration in the same state, instead of reading from a static list of values which requires intermediate caching.

One possible solution to the above described issues is to split up Viper's API:

  • Loader component should be responsible for fetching values from various sources (defaults and value overrides should be part of this component as well).
  • Config/State component is a merged list of values for fetching and setting values.
  • Dumper component would be responsible for writing the State to the filesystem for example.
  • Watcher component would watch for changes, trigger the Loader to fetch config from all sources, generate a new State and provide it to the callback.

Whether this is the right separation or not: I'm not sure.

Especially this last bit with the Watcher and the Loader component.

The obvious advantage of this separation is that the different groups will no longer be able step on each others toes.

On top of this lower level API we could preserve a facade-like object. With the proper separation, keeping concurrent actions apart would be easier than it is now.

Related issues

List of issues outlining problems caused by the current API:

Subtasks

List of actual tasks related to this issue:

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

No branches or pull requests

1 participant