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

Replace Viper as configuration manager #12

Merged
merged 3 commits into from
Jan 6, 2025
Merged

Conversation

davidmasek
Copy link
Owner

Viper as a configuration manager has it's problems, see https://github.com/knadh/koanf?tab=readme-ov-file#alternative-to-viper for some discussion.

For our use-case it is especially relevant that Viper does not handle keys without content well (i.e. it ignores them). But for Beacon service without configuration is still significant - since we want to know the name of the service that should be monitored even if no config is provided (we just supply the default one). See:

It seems like we need different solutions. Alternatives:

Since we require somewhat special parsing anyway (dynamic content for the config file, we already parse services pretty much on our own) I think it would make sense to just create our own parser with https://github.com/go-yaml/yaml .

  1. Create a module to store config functionality. config seems like a clear choice but would shadow variables of this name. configurer would help with this, but better name would be welcome.
  2. Implement the functionality to parse YAML file.
  3. Implement the functionality to overwrite values with ENV variables.
  4. Remove Viper and change usage to new configurer thingy in the codebase.

@davidmasek davidmasek marked this pull request as draft December 29, 2024 22:21
@davidmasek davidmasek marked this pull request as ready for review January 6, 2025 19:59
@davidmasek davidmasek merged commit 446a250 into main Jan 6, 2025
2 checks passed
@davidmasek davidmasek deleted the feature/bye-viper branch January 6, 2025 20:04
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.

1 participant