chore: use cobra and viper for local-artifact-mirror #1487
Conversation
|
This PR has been released (on staging) and is available for download with a embedded-cluster-smoke-test-staging-app license ID. Online Installer: Airgap Installer (may take a few minutes before the airgap bundle is built): Happy debugging! |
marccampbell
force-pushed
the
lam-cobra-viper
branch
2 times, most recently
from
204051f to
e0e3f8b
Compare
marccampbell
changed the title
marccampbell
force-pushed
the
lam-cobra-viper
branch
4 times, most recently
from
dba9c60 to
de5ec7d
Compare
marccampbell
force-pushed
the
lam-cobra-viper
branch
from
0386970 to
ccecd84
Compare
| return nil | ||
| }, | ||
| RunE: func(cmd *cobra.Command, args []string) error { | ||
| v := viper.GetViper() |
| return nil | ||
| }, | ||
| RunE: func(cmd *cobra.Command, args []string) error { | ||
| v := viper.GetViper() |
| return nil | ||
| }, | ||
| RunE: func(cmd *cobra.Command, args []string) error { | ||
| v := viper.GetViper() |
| if err != nil { | ||
| panic(fmt.Errorf("unable to get provider from filesystem: %w", err)) | ||
| RunE: func(cmd *cobra.Command, args []string) error { | ||
| v := viper.GetViper() |
| RunE: func(cmd *cobra.Command, args []string) error { | ||
| cmd.Help() | ||
| os.Exit(1) | ||
| return nil | ||
| }, |
There was a problem hiding this comment.
i feel like you can omit this entirely but maybe you tried that
| // Support for older env vars | ||
| flag := cmd.Flags().Lookup("data-dir") | ||
| if flag == nil || !flag.Changed { | ||
| if os.Getenv("LOCAL_ARTIFACT_MIRROR_DATA_DIR") != "" { | ||
| dataDir = os.Getenv("LOCAL_ARTIFACT_MIRROR_DATA_DIR") | ||
| } | ||
| } |
There was a problem hiding this comment.
could you make a convenience function for this since its repeated 3 times?
| v := viper.GetViper() | ||
|
|
||
| // there are some env vars that we still support for backwards compatibility | ||
| if os.Getenv("LOCAL_ARTIFACT_MIRROR_PORT") != "" { |
There was a problem hiding this comment.
Have you tried v.BindEnv for this?
| } | ||
| port = int(envvarPort) | ||
| } | ||
| if os.Getenv("LOCAL_ARTIFACT_MIRROR_DATA_DIR") != "" { |
There was a problem hiding this comment.
Have you tried v.BindEnv for this?
| if v.Get("data-dir") != nil { | ||
| provider = defaults.NewProvider(v.GetString("data-dir")) |
There was a problem hiding this comment.
This does not respect the env var LOCAL_ARTIFACT_MIRROR_DATA_DIR. You need to use the dataDir variable
There was a problem hiding this comment.
i think we could really benefit from moving the flag and env var parsing to functions and write unit tests
| port := v.GetInt("port") | ||
| if port == 0 { |
There was a problem hiding this comment.
i think we could really benefit from moving the flag and env var parsing to functions and write unit tests
This reverts commit fbc6db8.
No description provided.