Skip to content

return error in oc set env RESOURCE when no env args are provided#10485

Merged
openshift-bot merged 2 commits intoopenshift:masterfrom
juanvallejo:jvallejo_fix-oc-set-env-when-no-env-passed
Aug 18, 2016
Merged

return error in oc set env RESOURCE when no env args are provided#10485
openshift-bot merged 2 commits intoopenshift:masterfrom
juanvallejo:jvallejo_fix-oc-set-env-when-no-env-passed

Conversation

@juanvallejo
Copy link
Copy Markdown
Contributor

@juanvallejo juanvallejo commented Aug 17, 2016

Related BZ: https://bugzilla.redhat.com/show_bug.cgi?id=1366559

The command oc set RESOURCE prompts a user that the resource has been
updated even when no other args are provided:

$ oc set env dc/database
deploymentconfig "database" updated

This patch makes sure that at least one pair of environment variable
arguments are passed before returning a successful message:

$ oc set env dc/database
error: at least one environment variable must be provided in the form of
KEY=VALUE

cc @fabianofranz

if err != nil {
return err
}
if len(env) == 0 {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doesn't this break --list?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm, it does, I could add a check to make sure --list is not set before returning this error

Related BZ: https://bugzilla.redhat.com/show_bug.cgi?id=1366559

The command `oc set RESOURCE` prompts a user that the resource has been
updated even when no other args are provided:

```
$ oc set env dc/database
deploymentconfig "database" updated
```

This patch makes sure that at least one pair of environment variable
arguments are passed:

```
$ oc set env dc/database
error: at least one environment variable must be provided
```
@juanvallejo juanvallejo force-pushed the jvallejo_fix-oc-set-env-when-no-env-passed branch from 95d690f to 51e2be6 Compare August 17, 2016 15:37
@juanvallejo
Copy link
Copy Markdown
Contributor Author

@liggitt ptal
Moved the env argument check to the end, before the success message is printed, so that it doesn't break flags.

@juanvallejo
Copy link
Copy Markdown
Contributor Author

[test]

@juanvallejo
Copy link
Copy Markdown
Contributor Author

@openshift-bot, the last build failed from the following flakes:

re[test]

1 similar comment
@juanvallejo
Copy link
Copy Markdown
Contributor Author

@openshift-bot, the last build failed from the following flakes:

re[test]

@openshift-bot
Copy link
Copy Markdown
Contributor

Evaluated for origin test up to 51e2be6

@openshift-bot
Copy link
Copy Markdown
Contributor

continuous-integration/openshift-jenkins/test SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test_pr_origin/8143/)

@fabianofranz
Copy link
Copy Markdown
Member

LGTM [merge]

@openshift-bot
Copy link
Copy Markdown
Contributor

openshift-bot commented Aug 18, 2016

continuous-integration/openshift-jenkins/merge SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test_pr_origin/8143/) (Image: devenv-rhel7_4868)

@openshift-bot
Copy link
Copy Markdown
Contributor

Evaluated for origin merge up to 51e2be6

@openshift-bot openshift-bot merged commit bb48e69 into openshift:master Aug 18, 2016
@juanvallejo juanvallejo deleted the jvallejo_fix-oc-set-env-when-no-env-passed branch August 18, 2016 18:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants