-
Notifications
You must be signed in to change notification settings - Fork 87
fix: simplify config get to read directly from viper #974
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2,8 +2,11 @@ package config | |
|
|
||
| import ( | ||
| "fmt" | ||
| "os" | ||
| "strings" | ||
|
|
||
| "github.com/spf13/cobra" | ||
| "github.com/spf13/viper" | ||
|
|
||
| "github.com/openshift/backplane-cli/pkg/cli/config" | ||
| ) | ||
|
|
@@ -21,33 +24,40 @@ func newGetCmd() *cobra.Command { | |
| } | ||
|
|
||
| func getConfig(cmd *cobra.Command, args []string) error { | ||
| config, err := config.GetBackplaneConfiguration() | ||
| configPath, err := config.GetConfigFilePath() | ||
| if err != nil { | ||
| return err | ||
| } | ||
|
|
||
| proxyURL := "" | ||
| if config.ProxyURL != nil { | ||
| proxyURL = *config.ProxyURL | ||
| if _, err = os.Stat(configPath); err != nil { | ||
| if !os.IsNotExist(err) { | ||
| return fmt.Errorf("unable to access config file %s: %w", configPath, err) | ||
| } | ||
| } else { | ||
| viper.SetConfigFile(configPath) | ||
| viper.SetConfigType("json") | ||
| if err := viper.ReadInConfig(); err != nil { | ||
| return err | ||
| } | ||
| } | ||
|
|
||
| switch args[0] { | ||
| case URLConfigVar: | ||
| fmt.Printf("%s: %s\n", URLConfigVar, config.URL) | ||
| fmt.Printf("%s: %s\n", URLConfigVar, viper.GetString(URLConfigVar)) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Looks like this changes The exist implementation has the effective URL is resolved from |
||
| case ProxyURLConfigVar: | ||
| fmt.Printf("%s: %s\n", ProxyURLConfigVar, proxyURL) | ||
| fmt.Printf("%s: %s\n", ProxyURLConfigVar, strings.Join(viper.GetStringSlice(ProxyURLConfigVar), ", ")) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Looks like this bypasses the existing |
||
| case SessionConfigVar: | ||
| fmt.Printf("%s: %s\n", SessionConfigVar, config.SessionDirectory) | ||
| fmt.Printf("%s: %s\n", SessionConfigVar, viper.GetString(SessionConfigVar)) | ||
| case PagerDutyAPIConfigVar: | ||
| fmt.Printf("%s: %s\n", PagerDutyAPIConfigVar, config.PagerDutyAPIKey) | ||
| fmt.Printf("%s: %s\n", PagerDutyAPIConfigVar, viper.GetString(PagerDutyAPIConfigVar)) | ||
| case GovcloudVar: | ||
| fmt.Printf("%s: %t\n", GovcloudVar, config.Govcloud) | ||
| fmt.Printf("%s: %t\n", GovcloudVar, viper.GetBool(GovcloudVar)) | ||
| case "all": | ||
| fmt.Printf("%s: %s\n", URLConfigVar, config.URL) | ||
| fmt.Printf("%s: %s\n", ProxyURLConfigVar, proxyURL) | ||
| fmt.Printf("%s: %s\n", SessionConfigVar, config.SessionDirectory) | ||
| fmt.Printf("%s: %s\n", PagerDutyAPIConfigVar, config.PagerDutyAPIKey) | ||
| fmt.Printf("%s: %t\n", GovcloudVar, config.Govcloud) | ||
| fmt.Printf("%s: %s\n", URLConfigVar, viper.GetString(URLConfigVar)) | ||
| fmt.Printf("%s: %s\n", ProxyURLConfigVar, strings.Join(viper.GetStringSlice(ProxyURLConfigVar), ", ")) | ||
| fmt.Printf("%s: %s\n", SessionConfigVar, viper.GetString(SessionConfigVar)) | ||
| fmt.Printf("%s: %s\n", PagerDutyAPIConfigVar, viper.GetString(PagerDutyAPIConfigVar)) | ||
| fmt.Printf("%s: %t\n", GovcloudVar, viper.GetBool(GovcloudVar)) | ||
| default: | ||
| return fmt.Errorf("supported config variables are %s, %s, %s, %s, & %s", URLConfigVar, ProxyURLConfigVar, SessionConfigVar, PagerDutyAPIConfigVar, GovcloudVar) | ||
| } | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This may overwrite environment configuration settings. For example : GitHub - backplane-cli config.