Skip to content
This repository has been archived by the owner on May 31, 2023. It is now read-only.

Add the configure ercole using TOML instead of JSON #50

Closed
amreo opened this issue Mar 2, 2020 · 4 comments · Fixed by #72
Closed

Add the configure ercole using TOML instead of JSON #50

amreo opened this issue Mar 2, 2020 · 4 comments · Fixed by #72
Assignees
Labels
enhancement New feature or request Investigate Require the study and investigation of the feature
Milestone

Comments

@amreo
Copy link
Member

amreo commented Mar 2, 2020

Is your feature request related to a problem? Please describe.
The JSON format doesn't support comments so I want to configure ercole with TOML instead of JSON.

Describe the solution you'd like
I want that when ercole starts, it (also) reads the configuration files written in TOML

@amreo
Copy link
Member Author

amreo commented Mar 5, 2020

I tried to modify the function readConfig for adding the support to this feature but the onion and mergo are buggy, don't fit well and are incomplete...

func readConfig() config.Configuration {
	var conf config.Configuration

	layers := make([]onion.Layer, 0)
	l1, err := onion.NewFileLayer("/opt/ercole/config.json", nil)
	if err == nil {
		layers = append(layers, l1)
	}
	l2, err := onion.NewFileLayer("/usr/share/ercole/config.json", nil)
	if err == nil {
		layers = append(layers, l2)
	}
	l3, err := onion.NewFileLayer("/etc/ercole.json", nil)
	if err == nil {
		layers = append(layers, l3)
	}
	home, _ := os.UserHomeDir()
	l4, err := onion.NewFileLayer(home+"/.ercole.json", nil)
	if err == nil {
		layers = append(layers, l4)
	}
	l5, err := onion.NewFileLayer("config.json", nil)
	if err == nil {
		layers = append(layers, l5)
	}
	l6, err := onion.NewFileLayer(extraConfigFile, nil)
	if err == nil {
		layers = append(layers, l6)
	}
	l7 := onion.NewEnvLayerPrefix("_", "ERCOLE")
	layers = append(layers, l7)
	o := onion.New(layers...)

	//Onion doesn't support a getall like function, so I use this trick to decode all fields to the struct
	v := reflect.ValueOf(&conf).Elem()
	t := v.Type()
	for i := 0; i < t.NumField(); i++ {
		val, ok := o.Get(t.Field(i).Name)
		if ok {
			fmt.Println("q_ui10")
			err := mergo.Map(v.Field(i).Addr().Interface(), val, mergo.WithOverride, mergo.WithTransformers(utils.Float64toIntTransfomer{}))
			if err != nil {
				fmt.Printf("%s %T\n", v.Field(i).Type().String(), val)
				panic(err)

			}
		}
	}

	config.PatchConfiguration(&conf)

	//Return the read configuration
	return conf
}
func (t Float64toIntTransfomer) Transformer(typ reflect.Type) func(dst, src reflect.Value) error {
	if typ.Kind() == reflect.Float32 || typ.Kind() == reflect.Float64 || typ.Kind() == reflect.Int {
		return func(dst, src reflect.Value) error {
			if dst.CanSet() {
				if dst.Kind() == reflect.Int {
					dst.SetInt(int64(src.Float()))
				} else {
					dst.Set(src)
				}
			}
			return nil
		}
	}
	return nil
}

See darccio/mergo#138 and goraz/onion#18

@amreo
Copy link
Member Author

amreo commented Mar 5, 2020

I need a configuration library that

@amreo amreo added the Investigate Require the study and investigation of the feature label Mar 6, 2020
@cirelli94 cirelli94 self-assigned this Mar 9, 2020
@cirelli94
Copy link
Contributor

Hi @amreo,
https://github.com/goraz/onion that you already tried:

  • support TOML
  • support merging configuration between the layers

It doesn't support multiple files in the same directory, but we can merge the files easily, thanks to TOML format.

What do you mean with your last point? "Support configuration file with different names"?

@amreo
Copy link
Member Author

amreo commented Mar 10, 2020

support merging configuration between the layers

Are you sure that it support merging configuration between the layers??????

What do you mean with your last point? "Support configuration file with different names"?

I mean that different file in different directory can have different names, for example ~/.ercole.json, /etc/ercole.json, config.json

Cobra, for example, doesn't suppor this feature. See
SetConfigName and AddConfigPath

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement New feature or request Investigate Require the study and investigation of the feature
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants