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 odo create project -o json output #1916

Merged

Conversation

cdrage
Copy link
Member

@cdrage cdrage commented Jul 17, 2019

Adds machine readable output for odo create project.

To test:

odo create project -o json

@cdrage cdrage added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. Required by Prow. label Jul 17, 2019
@cdrage
Copy link
Member Author

cdrage commented Jul 17, 2019

/hold

This is WIP as well as builds on the #1898 branch..

@openshift-ci-robot openshift-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. Required by Prow. label Jul 17, 2019
@cdrage cdrage force-pushed the add-create-project branch 3 times, most recently from 048d279 to 4fea5cb Compare July 19, 2019 19:31
@cdrage
Copy link
Member Author

cdrage commented Jul 19, 2019

/retest

@cdrage cdrage force-pushed the add-create-project branch 5 times, most recently from d2c998b to ab68796 Compare July 29, 2019 15:48
@cdrage cdrage removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. Required by Prow. label Aug 2, 2019
@cdrage
Copy link
Member Author

cdrage commented Aug 2, 2019

/hold cancel

@openshift-ci-robot openshift-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. Required by Prow. label Aug 2, 2019
@cdrage cdrage force-pushed the add-create-project branch 4 times, most recently from ee0a080 to 40de901 Compare August 2, 2019 21:15
@mik-dass
Copy link
Contributor

mik-dass commented Aug 5, 2019

on Travis

✓  Waiting for project to come up [445ms]
✓  Project 'vvpzytcnxg' is ready for use
I0802 21:26:24.072718   18053 cmdutils.go:26] Error:
unable to set current project tovvpzytcnxg: unable to switch to vvpzytcnxg project: yaml: line 8: could not find expected ':'
✗  yaml: line 8: could not find expected ':'
Deleting project: 
Running odo with args [odo project delete  -f]
✗  Error loading config file "/home/travis/.kube/config": yaml: line 8: could not find expected ':'
 Please login to your server: 
837

On openshift CI


    Expected '' should be valid JSON, but it is not.
      
    Underlying error:unexpected end of JSON input
     /go/src/github.com/openshift/odo/tests/integration/json_test.go:49

/retest

@mik-dass
Copy link
Contributor

mik-dass commented Aug 6, 2019

Running odo with args [odo app delete app -f]
[odo]  ✗  invalid configuration: [context was not found for specified context: anbdgyfyxu/api-ci-op-dy725ykj-f09f4-origin-ci-int-aws-dev-rhcloud-com:6443/developer, cluster has no server defined]

/retest

@mik-dass
Copy link
Contributor

mik-dass commented Aug 6, 2019

✗  invalid configuration: [context was not found for specified context: anbdgyfyxu/api-ci-op-dy725ykj-f09f4-origin-ci-int-aws-dev-rhcloud-com:6443/developer, cluster has no server defined]

/retest


// If -o json has been passed, let's output the approriate json output!
if log.IsJSON() {
project.MachineReadableSuccessOutput(pco.projectName, successMessage)
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: this is a bit confusing as it feels that function is not breaking and the next line would get executed

Copy link
Member Author

Choose a reason for hiding this comment

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

Got it :) Added an else statement.

@girishramnani
Copy link
Contributor

girishramnani commented Aug 14, 2019

Other than what others have commented I have one nitpick

@girishramnani
Copy link
Contributor

/approve

@openshift-ci-robot
Copy link
Collaborator

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: girishramnani

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-ci-robot openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. Required by Prow. label Aug 14, 2019
Copy link
Contributor

@mik-dass mik-dass left a comment

Choose a reason for hiding this comment

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

/lgtm from me

/hold please have a look at @dharmit comments

@cdrage cdrage force-pushed the add-create-project branch 3 times, most recently from 9c9f18e to 080d774 Compare August 15, 2019 21:07
@cdrage
Copy link
Member Author

cdrage commented Aug 15, 2019

Could you have another quick look @dharmit ? Thanks for the updates!

@dharmit
Copy link
Member

dharmit commented Aug 20, 2019

@cdrage I still see:

$ odo project create -w newproject -o json | jq .
parse error: Invalid numeric literal at line 1, column 5

is this expected? #1916 (comment) makes me believe that you tried to fix this.

@cdrage
Copy link
Member Author

cdrage commented Aug 20, 2019

I'll investigate it again @dharmit for some reason, it is only being affected when using the --wait / -w command..

@dharmit
Copy link
Member

dharmit commented Aug 21, 2019

I'll investigate it again @dharmit for some reason, it is only being affected when using the --wait / -w command..

Are you going to do it as a part of this PR or create a separate issue to track it? I was told that latter would be helpful in unblocking #1960. We can cancel the hold in that case.

@cdrage
Copy link
Member Author

cdrage commented Aug 21, 2019

Just updated the PR again to fix your issue. Seems like a spinner was still being shown @dharmit

@cdrage
Copy link
Member Author

cdrage commented Aug 21, 2019

/retest all

Adds machine readable output for odo create project.

To test:

```sh
odo create project -o json
```
@dharmit
Copy link
Member

dharmit commented Aug 22, 2019

Works like a charm

$ odo project create -w newproject -o json | jq .
{
  "kind": "Project",
  "apiVersion": "odo.openshift.io/v1alpha1",
  "metadata": {
    "name": "newproject",
    "namespace": "newproject",
    "creationTimestamp": null
  },
  "message": "Project 'newproject' is ready for use"
}

Thanks @cdrage!

/hold cancel

@cdrage
Copy link
Member Author

cdrage commented Aug 22, 2019

/retest

@dharmit
Copy link
Member

dharmit commented Aug 22, 2019

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. Required by Prow. label Aug 22, 2019
@openshift-bot
Copy link

/retest

Please review the full test history for this PR and help us cut down flakes.

@cdrage
Copy link
Member Author

cdrage commented Aug 22, 2019

Looks like it's stick in limbo on testing...

/retest all

@openshift-merge-robot openshift-merge-robot merged commit 387145f into redhat-developer:master Aug 22, 2019
@cdrage cdrage deleted the add-create-project branch January 14, 2022 14:50
@rm3l rm3l added estimated-size/L (20-40) Rough sizing for Epics. About 2 sprints of work for a person. and removed size/L labels Jun 18, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. Required by Prow. estimated-size/L (20-40) Rough sizing for Epics. About 2 sprints of work for a person. lgtm Indicates that a PR is ready to be merged. Required by Prow.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

10 participants