Skip to content

Commit

Permalink
chore: remove unnecessary binding of root cmd flags (keploy#1715)
Browse files Browse the repository at this point in the history
Signed-off-by: gouravkrosx <[email protected]>
  • Loading branch information
gouravkrosx authored Mar 20, 2024
1 parent 895f699 commit fd210aa
Show file tree
Hide file tree
Showing 3 changed files with 18 additions and 11 deletions.
19 changes: 11 additions & 8 deletions cli/provider/cmd.go
Original file line number Diff line number Diff line change
Expand Up @@ -209,26 +209,29 @@ func (c *CmdConfigurator) AddFlags(cmd *cobra.Command, cfg *config.Config) error
utils.LogError(c.logger, err, errMsg)
return errors.New(errMsg)
}
err := viper.BindPFlag("debug", cmd.PersistentFlags().Lookup("debug"))
if err != nil {
errMsg := "failed to bind flag to config"
utils.LogError(c.logger, err, errMsg)
return errors.New(errMsg)
}
default:
return errors.New("unknown command name")
}
return nil
}

func (c CmdConfigurator) ValidateFlags(ctx context.Context, cmd *cobra.Command, cfg *config.Config) error {
func (c *CmdConfigurator) ValidateFlags(ctx context.Context, cmd *cobra.Command, cfg *config.Config) error {
// used to bind common flags for commands like record, test. For eg: PATH, PORT, COMMAND etc.
err := viper.BindPFlags(cmd.Flags())
utils.BindFlagsToViper(c.logger, cmd, "")
if err != nil {
errMsg := "failed to bind flags to config"
utils.LogError(c.logger, err, errMsg)
return errors.New(errMsg)
}

//used to bind flags specific to the command for eg: testsets, delay, recordTimer etc. (nested flags)
err = utils.BindFlagsToViper(c.logger, cmd, "")
if err != nil {
errMsg := "failed to bind cmd specific flags to viper"
utils.LogError(c.logger, err, errMsg)
return errors.New(errMsg)
}

if cmd.Name() == "test" || cmd.Name() == "record" {
configPath, err := cmd.Flags().GetString("configPath")
if err != nil {
Expand Down
4 changes: 2 additions & 2 deletions cli/provider/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,15 +41,15 @@ func NewServiceProvider(logger *zap.Logger, configDb *configdb.ConfigDb) *Servic
}

func (n *ServiceProvider) GetTelemetryService(ctx context.Context, config config.Config) (*telemetry.Telemetry, error) {
installtionID, err := n.configDb.GetInstallationID(ctx)
installationID, err := n.configDb.GetInstallationID(ctx)
if err != nil {
return nil, errors.New("failed to get installation id")
}
return telemetry.NewTelemetry(n.logger, telemetry.Options{
Enabled: !config.DisableTele,
Version: utils.Version,
GlobalMap: map[string]interface{}{},
InstallationID: installtionID,
InstallationID: installationID,
},
), nil
}
Expand Down
6 changes: 5 additions & 1 deletion utils/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,8 @@ import (

var WarningSign = "\U000026A0"

func BindFlagsToViper(logger *zap.Logger, cmd *cobra.Command, viperKeyPrefix string) {
func BindFlagsToViper(logger *zap.Logger, cmd *cobra.Command, viperKeyPrefix string) error {
var bindErr error
cmd.Flags().VisitAll(func(flag *pflag.Flag) {
// Construct the Viper key and the env variable name
if viperKeyPrefix == "" {
Expand All @@ -43,14 +44,17 @@ func BindFlagsToViper(logger *zap.Logger, cmd *cobra.Command, viperKeyPrefix str
err := viper.BindPFlag(viperKey, flag)
if err != nil {
LogError(logger, err, "failed to bind flag to config")
bindErr = err
}

// Tell Viper to also read this flag's value from the corresponding env variable
err = viper.BindEnv(viperKey, envVarName)
if err != nil {
LogError(logger, err, "failed to bind environment variables to config")
bindErr = err
}
})
return bindErr
}

//func ModifyToSentryLogger(ctx context.Context, logger *zap.Logger, client *sentry.Client, configDb *configdb.ConfigDb) *zap.Logger {
Expand Down

0 comments on commit fd210aa

Please sign in to comment.