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

Adds a flag to allow skipping config writes when creating a project #10057

Merged
merged 1 commit into from
Aug 3, 2016

Conversation

fabianofranz
Copy link
Member

Fixes #9979.

By using the --skip-config-write flag to skip the kubeconfig load and write, oc new-project will only call the REST API to create the project and exit. The command runs much faster, which is useful when creating multiple projects in a batch.

@fabianofranz
Copy link
Member Author

PTAL @jwforres @deads2k

[test]

}
}

fmt.Fprintf(o.Out, `
Copy link
Contributor

Choose a reason for hiding this comment

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

Looks like this is unconditional. Wouldn't it be kind of weird to output this to someone whose config is already updated and they're using this project?

Copy link
Member Author

Choose a reason for hiding this comment

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

oops, fixed.

@fabianofranz
Copy link
Member Author

re[test]

o.ProjectOptions.ProjectOnly = true
o.ProjectOptions.SkipAccessValidation = true
if !o.SkipConfigWrite {
if o.ProjectOptions != nil {
Copy link
Contributor

Choose a reason for hiding this comment

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

reading more carefully, couldn't SkipConfigWrite cause us to avoid creating the ProjectOptions in complete, which would prevent us from running RunProject, which would prevent the config write?

If you did that, then our nesting here stays the same, the existing Fprintf shoudl already have been under the existing if block, and you're just adding an else saying you can switch if you want?

if projectoptions != nil{
    // do existing working
    printf "you can add applications"
} else{
    printf "to switch to this project"
}

Copy link
Member Author

Choose a reason for hiding this comment

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

I think that makes sense. I'm using ProjectOptions to get the server host and be able to display ...created on server https://foo, but I think it can be taken from o.Client. Let me give it a try.

Copy link
Member Author

Choose a reason for hiding this comment

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

@deads2k Done. I still have to take a clientConfig to get the host, but definitely looks better.

@deads2k
Copy link
Contributor

deads2k commented Jul 28, 2016

@juanvallejo want a shot at it?

@fabianofranz fabianofranz force-pushed the issues_9979 branch 2 times, most recently from d7a1b38 to 42f14ee Compare July 28, 2016 15:48
@juanvallejo
Copy link
Contributor

@deads2k Sure, I'll go ahead and assign myself to review

@juanvallejo juanvallejo self-assigned this Jul 28, 2016
@fabianofranz fabianofranz force-pushed the issues_9979 branch 5 times, most recently from ac6bccd to 8e16cc8 Compare July 28, 2016 21:02
@juanvallejo
Copy link
Contributor

juanvallejo commented Jul 28, 2016

lgtm, is the extra tab in 8e16cc8#diff-87101210f21d81738e8b13078e66af90R137 intentional?

EDIT: Ah, I see why now, missed the fact that it was part of the fmt.Fprintf in the line above

@deads2k
Copy link
Contributor

deads2k commented Jul 29, 2016

lgtm [merge]

@fabianofranz
Copy link
Member Author

flake, re[merge]

@openshift-bot
Copy link
Contributor

openshift-bot commented Jul 30, 2016

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

@stevekuznetsov
Copy link
Contributor

re[test]
re[merge]

@fabianofranz fabianofranz force-pushed the issues_9979 branch 2 times, most recently from f4f23f3 to 426f8cf Compare August 2, 2016 14:24
@openshift-bot openshift-bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Aug 2, 2016
@openshift-bot
Copy link
Contributor

Evaluated for origin test up to 6ef4c5f

@openshift-bot openshift-bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Aug 2, 2016
@openshift-bot
Copy link
Contributor

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

@openshift-bot
Copy link
Contributor

Evaluated for origin merge up to 6ef4c5f

@openshift-bot openshift-bot merged commit be44a8a into openshift:master Aug 3, 2016
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.

5 participants