zeromicro / go-zero

A cloud-native Go microservices framework with cli tool for productivity.
https://go-zero.dev
MIT License
29.38k stars 3.97k forks source link

Unable to set default values in config #2266

Closed a0v0 closed 5 months ago

a0v0 commented 2 years ago

I am unable to set default value to config

etc/config.yaml

Media:

  MediaImageMaxSize: ${MEDIA_IMAGE_MAX_SIZE}

  MediaVideoMaxSize: ${MEDIA_VIDEO_MAX_SIZE}

  MediaDescriptionMaxLength: ${MEDIA_DESCRIPTION_MAX_LENGTH}

  MediaVideoTitleMaxLength: ${MEDIA_TITLE_MAX_LENGTH}

config.go

Media struct {
        MediaImageMaxSize         int64 `json:",default=5242880"`
        MediaVideoMaxSize         int64 `json:",default=41943040"`
        MediaDescriptionMaxLength int64 `json:",default=150"`
        MediaVideoTitleMaxLength  int64 `json:",default=25"`
    }

But I am getting this error

2022/08/15 23:17:28 error: config file etc/config.yaml, error: type mismatch for field Media.MediaImageMaxSize
exit status 1

also vscode shows warning as shown in the image. I think values from default tag are not being read. Any idea?

image

a0v0 commented 2 years ago

@kevwan currently if want to read a value from env we have to first specify it in etc/config.yml.

I think it would be better if we provide the default values in the etc/config.yml. To override an env var we could just pass an env var through env file.

if the configuration in config.yml is like this

Media:
    ImageSize: 24444

then gozero should expect MEDIA_IMAGE_SIZE as an env var.

kevwan commented 2 years ago

Good suggestion!

kevwan commented 2 years ago
MediaImageMaxSize         int64 `json:",default=5242880,env=MEDIA_IMAGE_SIZE"`

Use it like this?

a0v0 commented 2 years ago

Yes this will work fine. Also if the env var defined should override the config values from yaml. Docker compose ask for env file and also secrets and other env cars can easily be injected as env var in systems say GitHub secrets so env var is more important

kevwan commented 2 years ago

For the scenario that if I wrote a yaml file, and env overwrites the settings, I think it might be not obvious for devs.

Why the behavior is not like what I specified in my config? Oh, it's changed by the env, which might be injected by the platform.

And I think it might not a good idea to implement this feature before we can figure out a perfect solution.

a0v0 commented 2 years ago
MediaImageMaxSize         int64 `json:",default=5242880,env=MEDIA_IMAGE_SIZE"`

Use it like this?

@kevwan I think this is the best way. For priority order I please have a look at this discussion https://stackoverflow.com/questions/11077223/what-order-of-reading-configuration-values

Priority order: Command line parameter > environment variables > local config file (in our case yaml)

Why env should have higher precedence? https://stackoverflow.com/questions/11077223/what-order-of-reading-configuration-values

a0v0 commented 2 years ago

@kevwan any update?

kevwan commented 2 years ago

I'm working on it.

a0v0 commented 2 years ago

I'm working on it.

ok

a0v0 commented 2 years ago

hey @kevwan, I found github.com/num30/config package

This has exact functionality I need, loading values from flags, config files, and env vars (all file formats supported by viper)

Also it has the following order of loading config files.

Prority: flags > env > config file

Also default can be specified by just setting default tags. default:"postgres" validate:"hostname"

also validations can be done by go-validation package.

I was able to restructure my go application with the following changes:

original main.go:

package main

// TODO write tests

import (
    "app/internal/config"
    "app/internal/internal/handler"
    "app/internal/internal/svc"

    "flag"
    "fmt"

    "github.com/zeromicro/go-zero/core/conf"
    "github.com/zeromicro/go-zero/rest"
)

var configFile = flag.String("f", "etc/backbone.yaml", "the config file")

func main() {
    flag.Parse()

    var c config.Config
    conf.MustLoad(*configFile, &c, conf.UseEnv())

    server := rest.MustNewServer(c.RestConf)
    defer server.Stop()

    ctx := svc.NewServiceContext(c)

    handler.RegisterHandlers(server, ctx)

    fmt.Printf("Starting server at %s:%d...\n", c.Host, c.Port)
    server.Start()
}

new main.go:

package main

import (
    "fmt"
    "log"

    cfg "app/internal/config"
    "app/internal/internal/handler"
    "app/internal/internal/svc"

    "github.com/num30/config"
    "github.com/zeromicro/go-zero/rest"
)

func main() {
    var c cfg.Config

    // search for config file in the etc folder without caring for the file extension
    // Prority: flags > env > config file
    err := config.NewConfReader("backbone").WithSearchDirs("./etc", ".").Read(&c)
    if err != nil {
        log.Fatalf("error reading config: %v", err)
    }

    // TODO remove this
    log.Printf("config: %+v", c)

    server := rest.MustNewServer(c.RestConf)
    defer server.Stop()

    ctx := svc.NewServiceContext(c)

    handler.RegisterHandlers(server, ctx)

    fmt.Printf("Starting server at %s:%d...\n", c.Host, c.Port)
    server.Start()
}

old config file

package config

import (
    "frisbane.com/backbone/pkg/storage"
    "github.com/zeromicro/go-zero/core/stores/cache"
    "github.com/zeromicro/go-zero/rest"
)

type Config struct {
    rest.RestConf
    Redis cache.CacheConf

    Postgres struct {
        DataSource string
    }
}

new config file

package config

import (
    "github.com/zeromicro/go-zero/core/stores/cache"
    "github.com/zeromicro/go-zero/rest"
)

type Config struct {
    // we need `mapstructure:",squash"` here to refer to the inside properties of the struct without refering to the struct itself
    rest.RestConf `mapstructure:",squash"`

    Redis       cache.CacheConf
    Postgres    PostgresConf
}

type PostgresConf struct {
    Host     string `default:"postgres" validate:"hostname"`
    Port     int64  `default:"5432"`
    User     string `default:"postgres"`
    Password string `default:"postgres"`
    DbName   string `default:"postgres"`
}

But when running I am getting this error: 2022/09/13 14:59:59 no cache nodes

I think it's because redis or rest config has default set with this format json:",default=0.0.0.0" but this new package want the format to be in the format default:"0.0.0.0".

These tags needs to be changed in go-zero itself. Would you mind taking a look

a0v0 commented 2 years ago
type Config struct {
    // we need `mapstructure:",squash"` here to refer to the inside properties of the struct without refering to the struct itself
    rest.RestConf `mapstructure:",squash"`

    Redis       cache.CacheConf
    Postgres    PostgresConf
}

type PostgresConf struct {
    Host     string `default:"postgres" validate:"hostname"`
    Port     int64  `default:"5432"`
    User     string `default:"postgres"`
    Password string `default:"postgres"`
    DbName   string `default:"postgres"`
}

the benefit with this approach is that we can define the defaults from within the structs and validate them. Though go-zero supports setting defaults but for some reasons I am not able to get it working. default values never show up when i try to log them.

Also we do not have to specify env in config file like ${SOME_VAR}. In the above struct suppose we want to to set DbName. We can specify an env var REDIS_DBNAME directly and set the value.

Also values are merged no matter provided as flags, config file or env file with this priority:

flags > env > config file

This new package is just awesome.

a0v0 commented 2 years ago

I logged the conf file. Here is the formatted version

{
   "Name":"",
   "Log":{
      "ServiceName":"",
      "Mode":"",
      "Encoding":"",
      "TimeFormat":"",
      "Path":"",
      "Level":"",
      "Compress":false,
      "KeepDays":0,
      "StackCooldownMillis":0,
      "MaxBackups":0,
      "MaxSize":0,
      "Rotation":""
   },
   "Mode":"",
   "MetricsUrl":"",
   "Prometheus":{
      "Host":"",
      "Port":0,
      "Path":""
   },
   "Telemetry":{
      "Name":"",
      "Endpoint":"",
      "Sampler":0,
      "Batcher":""
   },
   "Host":"0.0.0.0",
   "Port":8080,
   "CertFile":"",
   "KeyFile":"",
   "Verbose":false,
   "MaxConns":0,
   "MaxBytes":0,
   "Timeout":10000,
   "CpuThreshold":0,
   "Signature":{
      "Strict":false,
      "Expiry":0,
      "PrivateKeys":null
   },
   "Redis":[
      {
         "Host":"",
         "Type":"",
         "Pass":"",
         "Tls":false,
         "Weight":0
      }
   ],

   "Postgres":{
      "Host":"postgres",
      "Port":5432,
      "User":"postgres",
      "Password":"postgres",
      "DbName":"postgres"
   },

}
Issues-translate-bot commented 2 years ago

Bot detected the issue body's language is not English, translate it automatically. πŸ‘―πŸ‘­πŸ»πŸ§‘β€πŸ€β€πŸ§‘πŸ‘«πŸ§‘πŸΏβ€πŸ€β€πŸ§‘πŸ»πŸ‘©πŸΎβ€πŸ€β€πŸ‘¨πŸΏπŸ‘¬πŸΏ


2382

github-actions[bot] commented 8 months ago

This issue is stale because it has been open for 30 days with no activity.

github-actions[bot] commented 5 months ago

This issue was closed because it has been inactive for 14 days since being marked as stale.