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

Invert configuration struct direction #2347

Open
fabxc opened this Issue Jan 17, 2017 · 6 comments

Comments

Projects
None yet
6 participants
@fabxc
Copy link
Member

fabxc commented Jan 17, 2017

Currently most of our components depend on package config as that's where their configuration structs are defined.
That's noisy for package reuse. In some places we work around it by specifying structs in the packages and mirroring them.

Package config should be similar to main in that it bundles together all packages and parses the YAML configuration into the package's own confiuguration structs.
We should also get rid of the YAML-unmarshal-embedded validation and instead provide decicate ValidateX methods.

@fabxc

This comment has been minimized.

Copy link
Member Author

fabxc commented Mar 2, 2017

Related to this, we should not parse YAML directly: http://ghodss.com/2014/the-right-way-to-handle-yaml-in-golang/

Instead use: https://github.com/ghodss/yaml – also how Kubernetes handles it.

What we have to watch out for is keeping the validation of "overflow" fields, which detects mostly typod keys by the user.

@juliusv

This comment has been minimized.

Copy link
Member

juliusv commented Mar 3, 2017

Interesting read. One thing to watch out for is whether error messages for invalid YAML (during the YAML->JSON conversion) will be better than the current ones. Some first tests seem promising, although the user might be confused about the mention of JSON conversion:

err: error converting YAML to JSON: yaml: [] mapping values are not allowed in this context at line 4, column 10
@brian-brazil

This comment has been minimized.

Copy link
Member

brian-brazil commented Mar 3, 2017

We've been bitten by the nil issue a few times, this does seem a rather roundabout way to solve it though.

@grobie

This comment has been minimized.

Copy link
Member

grobie commented Mar 3, 2017

I'm not sold on the idea of using https://github.com/ghodss/yaml yet. The premise of the blog post is "if you support both json and yaml, here is a way to skip yaml parsing". We only support yaml though, in contrast to Kubernetes.

While it's true that JSON is a subset of YAML (something I've told people a few times when they complained about yaml syntax), the reverse is not true. How are YAML references and anchors be handled for example?

@lswith

This comment has been minimized.

Copy link

lswith commented Mar 14, 2017

why not use something like viper? It's a global registry for your configuration. You can define your own configuration structs in each package and then simply rely on viper to populate it for you accordingly.

@shane-axiom

This comment has been minimized.

Copy link

shane-axiom commented Oct 18, 2018

Since this is the only open issue referencing the inability to use YAML anchors and map merging, I'll add my +1 for prometheus support of those features here. I've seen recommendations by devs to use external configuration management tools such as Viper here and in other issues, but IMO that approach is overkill for many use cases and some areas of config (e.g. alert rules) would benefit greatly from the ability to reuse previously defined maps.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.