spf13 / viper

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

Choosing config file with correct extension/configType #1163

Open jqk opened 3 years ago

jqk commented 3 years ago

Expected behavior (what you expected to happen):

search config file according to specified configType.

Actual behavior (what actually happened):

I have 2 config files config.json and config.yaml, in the same path at the same time. Only file extensions are different.

I got json config file when I set empty string or 'json' for configType. That's what I expected. But I still got json file when I set configType to 'yaml'.

Repl.it link:

Code reproducing the issue:

viper.SetConfigName("config") 
viper.SetConfigType("yaml")

For above code, I got config.json opened. That's wrong for me.

I can open config.yaml when running code below:

viper.SetConfigName("config.yaml") 
viper.SetConfigType("yaml")

I found what's happened in source code.

func (v *Viper) searchInPath(in string) (filename string) {
    jww.DEBUG.Println("Searching for config in ", in)
    for _, ext := range SupportedExts {
        jww.DEBUG.Println("Checking for", filepath.Join(in, v.configName+"."+ext))
        if b, _ := exists(v.fs, filepath.Join(in, v.configName+"."+ext)); b {
            jww.DEBUG.Println("Found: ", filepath.Join(in, v.configName+"."+ext))
            return filepath.Join(in, v.configName+"."+ext)
        }
    }

    if v.configType != "" {
        if b, _ := exists(v.fs, filepath.Join(in, v.configName)); b {
            return filepath.Join(in, v.configName)
        }
    }

    return ""
}

Put the code below prior to 'for _, ext' section will solve the problem.

    if v.configType != "" {
        if b, _ := exists(v.fs, filepath.Join(in, v.configName+"."+v.configType)); b {
            return filepath.Join(in, v.configName+"."+v.configType)
        }
    }

Environment:

Anything else we should know?:

None

github-actions[bot] commented 3 years 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! ❤️

phinnaeus commented 3 years ago

Ah, I had the same question but I think I understand what's going on here. SetConfigType doesn't do what you think it does. It isn't for specifying what extension the config file will have on disk, it's for specifying how to parse a config file which doesn't have any extension at all.

I think what this should actually be is a feature request for SetConfigExtension or something like that. Overloading ConfigType is messy, though maybe in v2 it could be renamed to ConfigFormat which I think is more meaningful.

sagikazarmark commented 3 years ago

The current workaround for this problem is reordering the SupportedExts that contains the file extensions Viper looks for.

The configuration for searching files is already complicated enough and I'm not sure adding another setter is a good solution. Let me think about it. Till then, see my suggested workaround.

photowey commented 2 years ago

Ah, I had the same question. version: v1.11.0.

the search folder contains toml and yml files with the same name.

configType: yml

search result: xxx.toml unmarshal error: cannot unmarshal !!seq into map[string]interface {}

piaodazhu commented 1 year ago

I also stepped into this pit today. My configuration filename is brain.yaml and my executable binary is brain, I set the ConfigType to yaml but Viper think that brain is a configuration file, then the program crashed.

My setting:

viper.SetConfigName("brain")
viper.SetConfigType("yaml")
viper.AddConfigPath("./")                              // current path. Unfortunately, my executable binary is here
viper.AddConfigPath("/etc/octopoda/")       // system etc path. Actually, my configuration is here now.
err := viper.ReadInConfig()
if err != nil {
panic("cannot read config because " + err.Error())    // error occurs. But I expect viper reading my conf under /etc/octopoda/
    }

I check the source code and found the problem may be here:

func (v *Viper) searchInPath(in string) (filename string) {
    v.logger.Debug("searching for config in path", "path", in)
    for _, ext := range SupportedExts {
        v.logger.Debug("checking if file exists", "file", filepath.Join(in, v.configName+"."+ext))
        if b, _ := exists(v.fs, filepath.Join(in, v.configName+"."+ext)); b {
            v.logger.Debug("found file", "file", filepath.Join(in, v.configName+"."+ext))
            return filepath.Join(in, v.configName+"."+ext)
        }
    }

    if v.configType != "" {
        if b, _ := exists(v.fs, filepath.Join(in, v.configName)); b {
            return filepath.Join(in, v.configName)
        }
    }

    return ""
}

Although some issues about this were discussed before, I still think it is not perfect: If I haven't called SetConfigType, maybe I want to search all possible configuration files (brain.yaml, brain.json, brain ...). Buf If I called SetConfigType before, why need Viper to search all supported extensions?

I understand that for some configuration file that has no extensions, configType is necessary for parsing. Maybe configType and configExtension can be separately set? For example:

viper.SetConfigName("foo")
viper.SetConfigExtension("yaml") // foo.yaml is acceptable
viper.SetConfigExtension("")         // foo is also acceptable. Without this line, foo won't be found by searchInPath()
viper.SetConfigType("yaml")         // both foo.yaml and foo should be unmarshalled as YAML
// viper.AddConfigPath ...

The above is my personal proposal, and I am willing to contribute if needed. Thanks to the authors for their work!

luweiqianyi commented 1 year ago

Ah, I had the same question. version: github.com/spf13/viper v1.16.0

the search folder contains toml and yml files with the same name.

configType: yml

search result: xxx.toml unmarshal error: cannot unmarshal !!seq into map[string]interface {}

I debug the code,find the problems

func (v *Viper) ReadInConfig() error {
    v.logger.Info("attempting to read in config file")
    filename, err := v.getConfigFile()
    if err != nil {
        return err
    }
 ... omit other code
}

the implementation of getConfigFile, will return the first file it finds, but in my code ,i have already call

viper.SetConfigName(fileName)
viper.SetConfigType(extension)

, it should return file name what i want, not the first file viper finds, hope to fix this problem

pdebakker-auguria commented 6 months ago

Issue is still here, just found it on v1.18.2

viper.SetConfigName("incoming_datastreams")
viper.SetConfigType("yaml")
viper.AddConfigPath(".")

This opened the incoming_datastreams.json instead of the incoming_datastreams.yaml file.

Xia-Liang commented 4 months ago

Try using viper.SetConfigFile() instead. I personally think viper.SetConfigType is toooooo misleading, bad func name

sagikazarmark commented 4 months ago

SetConfigFile is the way to go if you want to use a specific config file (ie. you know the path to the file with extension).

SetConfigName is for searching files of all supported types.

SetConfigType is used as an override (as well as a required call when using ReadConfig).