-
Notifications
You must be signed in to change notification settings - Fork 20
Add support of non-secret configs #1619
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
base: main
Are you sure you want to change the base?
Conversation
| configSetCmd.Flags().BoolP("name", "n", false, "name of the config (backwards compat)") | ||
| configSetCmd.Flags().BoolP("env", "e", false, "set the config from an environment variable") | ||
| configSetCmd.Flags().Bool("random", false, "set a secure randomly generated value for config") | ||
| configSetCmd.Flags().Bool("secret", true, "set a secret config") |
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.
Boolean flags with true as default are a little awkward since the only way to really use it is to say --secret=false, which can perhaps be considered a feature since you make it explicit that a config can be read?
|
|
||
| configCmd.AddCommand(configSetCmd) | ||
|
|
||
| configGetCmd.Flags().BoolP("name", "n", false, "name of the config") |
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.
I'd do this using positional arg(s). For a while we only had -n so I had changed that to be a boolean, so it's effectively ignored.
| configGetCmd.Flags().BoolP("name", "n", false, "name of the config") |
| // Exec(ctx context.Context, taskID TaskID, args ...string) error | ||
| GetInfo(ctx context.Context, taskID TaskID) (*TaskInfo, error) | ||
| PutSecret(ctx context.Context, name, value string) error | ||
| PutSecret(ctx context.Context, encrypt bool, name, value string) error |
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.
Actually, they are always encrypted, regardless of whether you can read back or not, so this is a misnomer.
Reverts #1618
Add back the non-secret config functionality.
This was previous approved but revert to allow for more testing time.