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.

Sign up for GitHub

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

Jump to bottom

crc config get returns an error code when the setting has a default value #3678

Closed
gbraad opened this issue May 24, 2023 · 5 comments · Fixed by #3694
Closed

crc config get returns an error code when the setting has a default value #3678

gbraad opened this issue May 24, 2023 · 5 comments · Fixed by #3694

Comments

@gbraad
Copy link
Contributor

gbraad commented May 24, 2023 •
edited
Loading

image
@gbraad gbraad converted this from a draft issue May 24, 2023
@cfergeau
Copy link
Contributor

2.18.0 and 2.10.0 both have the same behaviour, so if this is a regression it's not a recent one.

@cfergeau
Copy link
Contributor

crc/cmd/crc/cmd/config/get.go

Lines 30 to 32 in 2468363

if v.IsDefault {
return fmt.Errorf("Configuration property '%s' is not set. Default value is '%s'", key, v.AsString())

@gbraad
Copy link
Contributor Author

gbraad commented May 24, 2023

This seems to be an inconsistency. Defaults are not an error IMNSHO ;-)

@jsliacan
Copy link
Contributor

2.18.0 and 2.10.0 both have the same behaviour, so if this is a regression it's not a recent one.

@cfergeau I think back then it was less obvious. It wouldn't have happened if you did this I think.

$ crc config set preset openshift
$ echo $?
0
$ crc config get preset
Configuration property 'preset' is not set. Default value is 'openshift'
$ echo $?
1

@gbraad
Copy link
Contributor Author

gbraad commented May 24, 2023

It is maybe not a regression, but an inconsistency; it makes no sense to error out when it is the default

cfergeau added a commit to cfergeau/crc that referenced this issue Jun 1, 2023
@cfergeau
config: Return exit code 0 when getting default value
f8ae2dc 
`crc config get xxx` prints:
```
Configuration property 'xxx' is not set. Default value is 'yyy'
```
when its value is not set, but the command exit code is 1.

Since '1' is returned for errors (`crc config get invalid-prop`), this
is not the correct exit code. This commit changes it to 0, which is the
same as successful `crc config get` calls when the value is not the
default one.

This fixes crc-org#3678
@cfergeau cfergeau mentioned this issue Jun 1, 2023
Merged
openshift-merge-robot pushed a commit that referenced this issue Jun 2, 2023
@cfergeau @openshift-merge-robot
config: Return exit code 0 when getting default value
476418f 
`crc config get xxx` prints:
```
Configuration property 'xxx' is not set. Default value is 'yyy'
```
when its value is not set, but the command exit code is 1.

Since '1' is returned for errors (`crc config get invalid-prop`), this
is not the correct exit code. This commit changes it to 0, which is the
same as successful `crc config get` calls when the value is not the
default one.

This fixes #3678
@github-project-automation github-project-automation bot moved this from Notes to Done in Project planning: crc Jun 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
Project planning: crc
Status: Done
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

config: Return exit code 0 when getting default value cfergeau/crc
3 participants
@gbraad @cfergeau @jsliacan