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

[BUG] Flytestdlib pflags doesn't bind to passed in default string slice #2119

Closed
2 tasks done
pmahindrakar-oss opened this issue Feb 2, 2022 · 1 comment · Fixed by flyteorg/flytestdlib#118
Closed
2 tasks done
Assignees
Labels
bug Something isn't working

Comments

@pmahindrakar-oss
Copy link
Contributor

pmahindrakar-oss commented Feb 2, 2022

Describe the bug

//go:generate pflags UpdateClusterConfig --default-var DefaultUpdateClusterConfig --bind-default-var
var (
	DefaultUpdateClusterConfig = &UpdateClusterConfig{
		DryRun: false,
	}
)

// UpdateClusterConfig used for updating clusters.
type UpdateClusterConfig struct {
	DryRun            bool   `json:"dryRun" pflag:",execute command without making any modifications."`
	ClusterConfigFile string `json:"clusterConfigFile" pflag:",file containing the update cluster config parameters."`
	// Using snake case here since
	EnabledPlugins    []string `json:"enabled_plugins" pflag:",comma separated list of plugins to be enabled on the cluster."`
	EnabledProjects   []string `json:"enabled_projects" pflag:",comma separated list of projects to be enabled on the cluster."`
}

generated pflagset

// GetPFlagSet will return strongly types pflags for all fields in UpdateClusterConfig and its nested types. The format of the
// flags is json-name.json-sub-name... etc.
func (cfg UpdateClusterConfig) GetPFlagSet(prefix string) *pflag.FlagSet {
	cmdFlags := pflag.NewFlagSet("UpdateClusterConfig", pflag.ExitOnError)
	cmdFlags.BoolVar(&DefaultUpdateClusterConfig.DryRun, fmt.Sprintf("%v%v", prefix, "dryRun"), DefaultUpdateClusterConfig.DryRun, "execute command without making any modifications.")
	cmdFlags.StringVar(&DefaultUpdateClusterConfig.ClusterConfigFile, fmt.Sprintf("%v%v", prefix, "clusterConfigFile"), DefaultUpdateClusterConfig.ClusterConfigFile, "file containing the update cluster config parameters.")
	cmdFlags.StringSliceVar(&[]string{}, fmt.Sprintf("%v%v", prefix, "enabled_plugins"), []string{}, "comma separated list of plugins to be enabled on the cluster.")
	cmdFlags.StringSliceVar(&[]string{}, fmt.Sprintf("%v%v", prefix, "enabled_projects"), []string{}, "comma separated list of projects to be enabled on the cluster.")
	return cmdFlags
}

Expected behavior

Expected

// GetPFlagSet will return strongly types pflags for all fields in UpdateClusterConfig and its nested types. The format of the
// flags is json-name.json-sub-name... etc.
func (cfg UpdateClusterConfig) GetPFlagSet(prefix string) *pflag.FlagSet {
	cmdFlags := pflag.NewFlagSet("UpdateClusterConfig", pflag.ExitOnError)
	cmdFlags.BoolVar(&DefaultUpdateClusterConfig.DryRun, fmt.Sprintf("%v%v", prefix, "dryRun"), DefaultUpdateClusterConfig.DryRun, "execute command without making any modifications.")
	cmdFlags.StringVar(&DefaultUpdateClusterConfig.ClusterConfigFile, fmt.Sprintf("%v%v", prefix, "clusterConfigFile"), DefaultUpdateClusterConfig.ClusterConfigFile, "file containing the update cluster config parameters.")
        cmdFlags.StringSliceVar(&DefaultUpdateClusterConfig.EnabledPlugins, fmt.Sprintf("%v%v", prefix, "enabledPlugins"), DefaultUpdateClusterConfig.EnabledPlugins, "comma separated list of plugins to be enabled on the cluster.")
	cmdFlags.StringSliceVar(&DefaultUpdateClusterConfig.EnabledProjects, fmt.Sprintf("%v%v", prefix, "enabledProjects"), DefaultUpdateClusterConfig.EnabledProjects, "comma separated list of projects to be enabled on the cluster.")
	return cmdFlags
}

Additional context to reproduce

No response

Screenshots

No response

Are you sure this issue hasn't been raised already?

  • Yes

Have you read the Code of Conduct?

  • Yes
@pmahindrakar-oss pmahindrakar-oss added bug Something isn't working untriaged This issues has not yet been looked at by the Maintainers labels Feb 2, 2022
@pmahindrakar-oss
Copy link
Contributor Author

pmahindrakar-oss commented Feb 2, 2022

cc : @EngHabu @katrogan

Currently working around this issue by manually editing the generated file and will take up this fix in a followup

@pmahindrakar-oss pmahindrakar-oss self-assigned this Feb 2, 2022
@EngHabu EngHabu added this to the 1.0.0 - Phoenix! milestone Feb 23, 2022
@EngHabu EngHabu removed the untriaged This issues has not yet been looked at by the Maintainers label Feb 23, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants