fabric8-services / fabric8-jenkins-proxy

Apache License 2.0
1 stars 16 forks source link

Replace handcoded configuration class with something better like viper #302

Closed kishansagathiya closed 4 years ago

kishansagathiya commented 6 years ago

All functions in the below code have the same body. We have more functions like this in the same file. Can we get rid of this redundancy and write in a better way?

https://github.com/fabric8-services/fabric8-jenkins-proxy/blob/83d6642a107f881be0444b7ae0c234269aefc9eb/internal/configuration/env_config.go#L167-L229

@alexeykazakov Would know anything about how to do this?

-- Updated

https://github.com/spf13/viper  does a much better job at handling app configuration, so lets remove the hand-coded one in favour of viper. The task need to ensure that we only pass relevant part of the config to each of the service and not the entire config object to every constructor.

sthaha commented 6 years ago

We could do something like this which I tried for fun

package main

import (
    "fmt"
    "os"
    "strconv"
)

type StringType func() string
type IntType func() int
type StringListType func() []string

func lookupEnv(env, defaultVal string) string {
    if value, ok := os.LookupEnv(env); ok {
        return value
    }
    return defaultVal
}

func String(key, fallback string) StringType {
    return func() string {
        if value, ok := os.LookupEnv(key); ok {
            return value
        }
        return fallback
    }
}

func Int(key string, fallback int) IntType {
    return func() int {

        value, ok := os.LookupEnv(key)
        if !ok {
            return fallback
        }

        if i, err := strconv.Atoi(value); err == nil {
            return i
        }
        return fallback
    }
}

type config struct {
    foo StringType
    bar StringType
    age IntType
}

func newConfig() *config {
    return &config{
        bar: String("BAR", "foobar"),
        foo: String("FOO", "foobarbaz"),
        age: Int("AGE", 10),
    }
}

func main() {
    c := newConfig()
    fmt.Println(c.foo())
    fmt.Println(c.bar())
    fmt.Println(c.age())
}
alexeykazakov commented 6 years ago

In some our Go services we use viper - https://github.com/spf13/viper For example in Auth - https://github.com/fabric8-services/fabric8-auth/blob/master/configuration/configuration.go In WIT - https://github.com/fabric8-services/fabric8-wit/blob/master/configuration/configuration.go

sthaha commented 6 years ago

In some our Go services we use viper - https://github.com/spf13/viper

thanks @alexeykazakov exactly the library I was searching for.