spf13 / viper

Go configuration with fangs
MIT License
27.36k stars 2.02k forks source link

viper.SetConfigFile() #1505

Open zouxingyuks opened 1 year ago

zouxingyuks commented 1 year ago

Preflight Checklist

Viper Version

1.15.0

Go Version

1.19.3

Config Source

Files

Format

YAML

Repl.it link

https://replit.com/@zouxingyuks/viper#main.go

Code reproducing the issue

package main

import (
    "fmt"
    "github.com/fsnotify/fsnotify"
    "github.com/spf13/viper"
    "os"
)

// 设置默认配置信息
var defaultConifg = []byte(`
Hacker: true
name: steve
hobbies:
- skateboarding
- snowboarding
- go
clothing:
  jacket: leather
  trousers: denim
age: 35
eyes : brown
beard: true
`)

func init() {
    parseConfig()
}
func main() {
    //在main函数中添加下列代码
    viper.WatchConfig()
    viper.OnConfigChange(func(e fsnotify.Event) {
        // 配置文件发生变更之后会调用的回调函数
        fmt.Println("Config file changed:", e.Name)
    })
    fmt.Println(viper.GetString("username"))
}
func parseConfig() {
    // 指定配置文件路径
    configPath := "./config.yaml"
    viper.SetConfigFile(configPath)
    if err := viper.ReadInConfig(); err != nil {
        // 配置文件出错
        //todo 设置日志输出
        if _, ok := err.(viper.ConfigFileNotFoundError); ok {
            // 配置文件未找到错误;如果需要可以忽略
            os.Create(configPath)
            //初始化配置文件

            //todo 补充日志创建说明
        } else {
            // 配置文件被找到,但产生了另外的错误
            test1, test2 := err.(viper.ConfigFileNotFoundError)
            fmt.Println(test1, test2)
            panic("The configuration file was found, but another error was generated")
        }
    }

    // 配置文件找到并成功解析
}

Expected Behavior

When the configuration file does not exist, the code that creates the file should be executed

Actual Behavior

A panic statement was executed

Steps To Reproduce

创建main.go package main

import ( "fmt" "github.com/fsnotify/fsnotify" "github.com/spf13/viper" "os" )

// 设置默认配置信息 var defaultConifg = []byte(` Hacker: true name: steve hobbies:

func init() { parseConfig() } func main() { //在main函数中添加下列代码 viper.WatchConfig() viper.OnConfigChange(func(e fsnotify.Event) { // 配置文件发生变更之后会调用的回调函数 fmt.Println("Config file changed:", e.Name) }) fmt.Println(viper.GetString("username")) } func parseConfig() { // 指定配置文件路径 configPath := "./config.yaml" viper.SetConfigFile(configPath) if err := viper.ReadInConfig(); err != nil { // 配置文件出错 //todo 设置日志输出 if _, ok := err.(viper.ConfigFileNotFoundError); ok { // 配置文件未找到错误;如果需要可以忽略 os.Create(configPath) //初始化配置文件

        //todo 补充日志创建说明
    } else {
        // 配置文件被找到,但产生了另外的错误
        test1, test2 := err.(viper.ConfigFileNotFoundError)
        fmt.Println(test1, test2)
        panic("The configuration file was found, but another error was generated")
    }
}

// 配置文件找到并成功解析

} 执行,成功应创建config.yaml

Additional Information

No response

github-actions[bot] commented 1 year ago

👋 Thanks for reporting!

A maintainer will take a look at your issue shortly. 👀

In the meantime: We are working on Viper v2 and we would love to hear your thoughts about what you like or don't like about Viper, so we can improve or fix those issues.

⏰ If you have a couple minutes, please take some time and share your thoughts: https://forms.gle/R6faU74qPRPAzchZ9

📣 If you've already given us your feedback, you can still help by spreading the news, either by sharing the above link or telling people about this on Twitter:

https://twitter.com/sagikazarmark/status/1306904078967074816

Thank you! ❤️

Ozoniuss commented 1 year ago

I'm having a similar issue, and I believe the problem is from the call to the SetConfigFile() function. It seems that if the config file is set via v.SetConfigFile(), then the error if the file is absent is of type *fs.PathError. See the output of the following code:

func main() {

    v := viper.New()
    v.SetConfigFile("./config.yaml")

    if err := v.ReadInConfig(); err != nil {
        if _, ok := err.(viper.ConfigFileNotFoundError); ok {
            fmt.Println("here")
            fmt.Println(err)
            fmt.Println(reflect.TypeOf(err))
        } else {
            fmt.Println("there")
            fmt.Println(err)
            fmt.Println(reflect.TypeOf(err))
        }
        os.Exit(1)
    }
    os.Exit(0)
}

output:

there
open ./config.yaml: The system cannot find the file specified.
*fs.PathError

However, the documentation suggests doing this, in which case the error works as expected:

func main() {

    v := viper.New()
    v.SetConfigName("config")
    v.SetConfigType("yaml")
    v.AddConfigPath(".")

    if err := v.ReadInConfig(); err != nil {
        if _, ok := err.(viper.ConfigFileNotFoundError); ok {
            fmt.Println("here")
            fmt.Println(err)
            fmt.Println(reflect.TypeOf(err))
        } else {
            fmt.Println("there")
            fmt.Println(err)
            fmt.Println(reflect.TypeOf(err))
        }
        os.Exit(1)
    }
    os.Exit(0)
}

output:

here
Config File "config" Not Found in "[C:\\redacted\\test2]"
viper.ConfigFileNotFoundError

Note that I manually redacted the full path

So I believe using the second example will work in your case.

However, I'm still confused by the fact that setting the config path explicitly gives a different error if the file is absent than setting paths for viper to look for the config. Is there a reason for this difference in behavior?

Viper version & Go version

My go.mod file ``` module test2 go 1.19 require ( github.com/fsnotify/fsnotify v1.6.0 // indirect github.com/hashicorp/hcl v1.0.0 // indirect github.com/magiconair/properties v1.8.7 // indirect github.com/mitchellh/mapstructure v1.5.0 // indirect github.com/pelletier/go-toml/v2 v2.0.6 // indirect github.com/spf13/afero v1.9.3 // indirect github.com/spf13/cast v1.5.0 // indirect github.com/spf13/jwalterweatherman v1.1.0 // indirect github.com/spf13/pflag v1.0.5 // indirect github.com/spf13/viper v1.15.0 // indirect github.com/subosito/gotenv v1.4.2 // indirect golang.org/x/sys v0.3.0 // indirect golang.org/x/text v0.5.0 // indirect gopkg.in/ini.v1 v1.67.0 // indirect gopkg.in/yaml.v3 v3.0.1 // indirect ) ```

EDIT: I've been looking a bit through the viper code to try and understand what the issue is. The call v.SetConfigFile(in string) sets the property v.configFile to the input string, if non-empty. Then, when calling v.ReadInConfig(), one of the first lines calls the getConfigFile() function, which either tries to find the config file using findConfigFile() or returns v.configFile directly if that is set.

Now, the call to findConfigFile() returns the viper error if no config file is found in the provided paths. However, if the file was specified via SetConfigFile and the property v.configFile is non-empty, ReadInConfig tries to read the file without checking that it exists:

file, err := afero.ReadFile(v.fs, filename)
if err != nil {
    return err
}

This returns the same error as fs.Open() if it can't open the file, which I assume is *fs.PathError.

I don't know if this is expected behavior, but if it is I would at least provide a sample usage of the SetConfigFile function, which is not present in the docs (I can contribute with more info to the docs in that case).

jamestiotio commented 1 year ago

It seems that if the config file is set via v.SetConfigFile(), then the error if the file is absent is of type *fs.PathError. ... However, I'm still confused by the fact that setting the config path explicitly gives a different error if the file is absent than setting paths for viper to look for the config. Is there a reason for this difference in behavior? ... I don't know if this is expected behavior, but if it is I would at least provide a sample usage of the SetConfigFile function, which is not present in the docs (I can contribute with more info to the docs in that case).

Hey @Ozoniuss, this part of your concerns was raised in #1491. The maintainers have not responded to that issue regarding whether it is expected behavior or not.

If it is expected behavior, the documentation can be updated to better reflect this. If it is not expected behavior, one change we could implement would be to modify the ReadInConfig function to first check if the file exists or not and raise ConfigFileNotFoundError if the file does not exist. Alternatively, we could just modify the returned error after the call to afero.ReadFile to ConfigFileNotFoundError (like the changes introduced in #1540). I am not sure if this is a breaking change though as some applications might have relied on the fact that *fs.PathError is returned. We would need the maintainers to chip in on this.

Ozoniuss commented 1 year ago

Yeah, agree, didn't see #1491, thanks for pointing it out. Looks like #1540 attempts to fix it, we'll see what the maintainers say about that.

sagikazarmark commented 1 year ago

Please see my comment in #1491