Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ReadConfig(in io.Reader) does not Actually ReadConfig #1784

Open
3 tasks done
dp1140a opened this issue Mar 15, 2024 · 3 comments
Open
3 tasks done

ReadConfig(in io.Reader) does not Actually ReadConfig #1784

dp1140a opened this issue Mar 15, 2024 · 3 comments
Labels
kind/bug Something isn't working

Comments

@dp1140a
Copy link

dp1140a commented Mar 15, 2024

Preflight Checklist

  • I have searched the issue tracker for an issue that matches the one I want to file, without success.
  • I am not looking for support or already pursued the available support channels without success.
  • I have checked the troubleshooting guide for my problem, without success.

Viper Version

1.18.2

Go Version

1.22

Config Source

Files

Format

TOML

Repl.it link

No response

Code reproducing the issue

/**
testdata/good_config.toml is a good toml file.
Viper sees it initially
**/
f, err := os.Open("testdata/good_config.toml")
if err != nil {
   assert.Fail(suite.T(), err.Error())
}
err = viper.ReadConfig(f)
if err != nil {
  assert.Fail(suite.T(), err.Error())
}

Expected Behavior

should read the config. When I print the config I expect the contents of the initial file. but I get {}

Actual Behavior

Return an empty config

Steps To Reproduce

No response

Additional Information

Did some initial debugging on this.

shot1
At this point all is well.

shot2
Here is where things start to hinky

shot3
Still dont have a file tto read? Ok says Viper Ill try to find the config file . . . But wait!

shot4
Here you can see that configPath has never been set. It still has not set the configFile or pconfigPath despite asking for a file reader that has the name and path of the file. The reult is that it cant find the config file and fails.

BUT!!!
This code works:

file := "testdata/good_config.toml"
viper.SetConfigName(strings.TrimSuffix(filepath.Base(file), filepath.Ext(file)))
viper.SetConfigType("toml")
viper.AddConfigPath(filepath.Dir(file))
f, err := os.Open(file)
if err != nil {
   assert.Fail(suite.T(), err.Error())
}
err = viper.ReadConfig(f)
if err != nil {
  assert.Fail(suite.T(), err.Error())
}

So why would I go through all that trouble to use ReadConfig(f) when I could just go through the same trouble and use ReadInConfig()?

Seems like ReadConfig(f) should just ReadConfig from f

@dp1140a dp1140a added the kind/bug Something isn't working label Mar 15, 2024
Copy link

👋 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! ❤️

@mazenharake
Copy link

mazenharake commented Mar 22, 2024

This seems to come up as a consequence of not specifying the config type. If you read in a file it still needs to be interpreted in some way and since the config type has not been set then getConfigType() resorts to trying to find the file instead and inferring the config type from the file extension. This is (probably) why your working example is successful.

See https://github.com/spf13/viper/blob/master/viper.go#L2218

I'll admit it is quirky behavior... perhaps adding a default case here https://github.com/spf13/viper/blob/master/viper.go#L1804 and retur an error would be more suitable since the function assumes that a config type has been set either explicitly using SetConfigType or by using SetConfigFile()+ReadInConfig()

Either way, using SetConfigType should resolve this issue?

@sagikazarmark
Copy link
Collaborator

I believe @mazenharake is correct.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants