Make --output a global parameter, disable logging, fix bug #1898

cdrage · 2019-07-10T17:26:41Z

This PR:

Makes -o or --output a global parameter
Disables logging completely when performing -o json, including
using verbosity -v
Fixes a minor bug within some commands where -o json wasn't
detected corretly due to a local flag being used outputFlag rather
than OutputFlag

cdrage · 2019-07-10T20:39:26Z

/hold

making some changes for debugging

cdrage · 2019-07-12T17:11:29Z

/retest

cdrage · 2019-07-16T18:32:23Z

Had to combine some of my commits, but this is ready for review 👍 @surajnarwade @kadel @mik-dass @amitkrout

amitkrout · 2019-07-17T06:15:41Z

#1782 (comment)
/test integration

mik-dass · 2019-07-17T08:29:56Z

[odo]  ✗  Failed to create component with name ref-test. Please use `odo config view` to view settings used to create component. Error: unable to wait for build ref-test-app-1 to run: timed out waiting for the condition

/retest

mik-dass · 2019-07-17T08:39:49Z

pkg/log/status.go

 	// set new status
 	isTerm := IsTerminal(s.writer)
 	s.status = status

 	// If we are in debug mode, don't spin!
 	if !isTerm || debug {
 		fmt.Fprintf(s.writer, prefixSpacing+getSpacingString()+suffixSpacing+"%s  ...\n", s.status)
-	} else {
+	} else if !json {


why not use IsJSON() function here too instead of passing a extra parameter?

You're right, I should of changed the old code here.. Thanks!

cdrage · 2019-07-17T18:33:31Z

@mik-dass thanks for the review, updated again!

cdrage · 2019-07-17T18:59:56Z

/retest

cdrage · 2019-07-17T19:03:07Z

pkg/odo/cli/cli.go

+	// We use "flag" in order to make this accessible throughtout ALL of odo, rather than the
+	// above traditional "persistentflags" usage that does not make it a pointer within the 'pflag'
+	// package
+	flag.CommandLine.String("o", "", "Specify output format, supported format: json")


@kadel With the way that global parameters are added with the standard flag library as well as the limitation on adding multiple flag aliases, either only -o OR --output can be added.

Would you be fine @kadel @dgolovin with removing --output (I dont see it being used much) with just -o ?

mik-dass · 2019-07-18T12:09:50Z

 waited 4m0s but couldn't find running pod matching selector: 'deploymentconfig=nodejs-push-atonce-nodejs-push-context-test

/retest

cdrage · 2019-07-18T21:26:04Z

/retest

mik-dass · 2019-07-19T06:41:57Z

[odo]  ✗  waited 4m0s but couldn't find running pod matching selector: 'deploymentconfig=nodejs-app'

/retest

mik-dass · 2019-07-26T07:11:11Z

/retest

mik-dass

/approve

openshift-ci-robot · 2019-07-26T11:49:42Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: mik-dass

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:

~~pkg/log/OWNERS~~ [mik-dass]
~~pkg/odo/OWNERS~~ [mik-dass]

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

cdrage · 2019-07-26T21:32:15Z

Tests finally pass 👍
@surajnarwade @kadel @girishramnani @amitkrout @dharmit @mohammedzee1000 @dharmit review?

This PR: - Makes `-o` or `--output` a global parameter - Disables logging completely when performing `-o json`, including using verbosity `-v` - Fixes a minor bug within some commands where `-o json` wasn't detected corretly due to a local flag being used `outputFlag` rather than `OutputFlag`

cdrage · 2019-07-29T19:21:40Z

/retest

mohammedzee1000

Codewise looking good. Not sure why we have failure related to looking for a value FOO.
https://prow.svc.ci.openshift.org/view/gcs/origin-ci-test/pr-logs/pull/openshift_odo/1898/pull-ci-openshift-odo-master-integration/1054#0:build-log.txt%3A8438
At initial glance, does not seem to be related to this code cgange

dharmit · 2019-07-30T07:33:55Z

Codewise looking good. Not sure why we have failure related to looking for a value FOO.
https://prow.svc.ci.openshift.org/view/gcs/origin-ci-test/pr-logs/pull/openshift_odo/1898/pull-ci-openshift-odo-master-integration/1054#0:build-log.txt%3A8438
At initial glance, does not seem to be related to this code cgange

The test in question was added by me as a part of #1860. I believe I should have modified the code to check for FOO instead of FOO: as is the case in couple of lines above at: https://github.com/openshift/odo/blob/d3a1084deccceb45b9010c757bc63a8f739b9e0b/tests/integration/component.go#L504

If this fails repeatedly at the same place, we can try doing that change.

amitkrout · 2019-07-30T08:04:46Z

/test integration

amitkrout · 2019-07-30T08:17:45Z

Codewise looking good. Not sure why we have failure related to looking for a value FOO.
https://prow.svc.ci.openshift.org/view/gcs/origin-ci-test/pr-logs/pull/openshift_odo/1898/pull-ci-openshift-odo-master-integration/1054#0:build-log.txt%3A8438
At initial glance, does not seem to be related to this code cgange

The test in question was added by me as a part of #1860. I believe I should have modified the code to check for FOO instead of FOO: as is the case in couple of lines above at:

https://github.com/openshift/odo/blob/d3a1084deccceb45b9010c757bc63a8f739b9e0b/tests/integration/component.go#L504

If this fails repeatedly at the same place, we can try doing that change.

@dharmit The issue is hitting due to line https://github.com/openshift/odo/blob/master/tests/integration/component.go#L507. Due to the parallel run on two test node oc describe is looking into some other dc currently active in some other name space.

stdOut = helper.CmdShouldPass("oc", "describe", "dc", "-n", project)

will fix the issue

amitkrout

This PR:
Makes -o or --output a global parameter

@cdrage Thanks for the pr. Can you please add some more context of why we need this change.

Disables logging completely when performing -o json, including
using verbosity -v

+1, make sense

Fixes a minor bug within some commands where -o json wasn't
detected corretly due to a local flag being used outputFlag rather
than OutputFlag

Looks like a bug fix. Can you guide the steps to reproduce it. I am asking because it will be helpful to analyze/write the possible test case if any.

surajnarwade

Codewise LGTM

/lgtm

cdrage · 2019-07-30T15:43:14Z

/hold cancel

cdrage · 2019-07-30T15:43:55Z

/refresh

openshift-ci-robot added the size/M label Jul 10, 2019

openshift-ci-robot requested review from girishramnani and surajnarwade July 10, 2019 17:27

openshift-ci-robot added size/L and removed size/M labels Jul 10, 2019

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 10, 2019

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 10, 2019

cdrage force-pushed the no-logging-when-json branch 3 times, most recently from 79140f1 to 4a729cf Compare July 12, 2019 15:01

cdrage mentioned this pull request Jul 15, 2019

Machine readable output changes (global changes + error output) #1892

Closed

cdrage force-pushed the no-logging-when-json branch 2 times, most recently from a3212be to 140c9fd Compare July 16, 2019 18:31

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 Jul 16, 2019

cdrage force-pushed the no-logging-when-json branch from 18fe8ef to b3355cb Compare July 16, 2019 18:57

mik-dass reviewed Jul 17, 2019

View reviewed changes

cdrage mentioned this pull request Jul 17, 2019

Review JSON output for all odo commands #1739

Closed

2 tasks

cdrage commented Jul 17, 2019

View reviewed changes

cdrage mentioned this pull request Jul 17, 2019

Adds odo create project -o json output #1916

Merged

cdrage force-pushed the no-logging-when-json branch from 19b134e to c030b08 Compare July 18, 2019 20:15

mik-dass approved these changes Jul 26, 2019

View reviewed changes

openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. Required by Prow. label Jul 26, 2019

cdrage force-pushed the no-logging-when-json branch 2 times, most recently from ee5b504 to f8f5e19 Compare July 26, 2019 19:47

cdrage force-pushed the no-logging-when-json branch from f8f5e19 to fb0fd8b Compare July 29, 2019 15:47

cdrage mentioned this pull request Jul 29, 2019

Add odo project delete -o json #1960

Closed

mohammedzee1000 reviewed Jul 30, 2019

View reviewed changes

amitkrout mentioned this pull request Jul 30, 2019

Fixing oc describe dc test falke #1963

Merged

amitkrout reviewed Jul 30, 2019

View reviewed changes

surajnarwade approved these changes Jul 30, 2019

View reviewed changes

openshift-ci-robot assigned surajnarwade Jul 30, 2019

openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. Required by Prow. label Jul 30, 2019

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 Jul 30, 2019

openshift-merge-robot merged commit dd181e6 into redhat-developer:master Jul 30, 2019

mohammedzee1000 mentioned this pull request Aug 6, 2019

Bumps version to v1.0.0-beta4 #1987

Merged

cdrage mentioned this pull request Aug 6, 2019

Don't output *any* info (other than json) when using -o json #1864

Closed

girishramnani mentioned this pull request Jan 2, 2020

Machine readable output - global -o flag #1475

Closed

cdrage deleted the no-logging-when-json branch January 14, 2022 14:53

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

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Make --output a global parameter, disable logging, fix bug #1898

Make --output a global parameter, disable logging, fix bug #1898

cdrage commented Jul 10, 2019

cdrage commented Jul 10, 2019

cdrage commented Jul 12, 2019

cdrage commented Jul 16, 2019

amitkrout commented Jul 17, 2019

mik-dass commented Jul 17, 2019

mik-dass Jul 17, 2019 •

edited

cdrage Jul 17, 2019

cdrage commented Jul 17, 2019

cdrage commented Jul 17, 2019

cdrage Jul 17, 2019

mik-dass commented Jul 18, 2019

cdrage commented Jul 18, 2019

mik-dass commented Jul 19, 2019

mik-dass commented Jul 26, 2019

mik-dass left a comment

openshift-ci-robot commented Jul 26, 2019

cdrage commented Jul 26, 2019

cdrage commented Jul 29, 2019

mohammedzee1000 left a comment

dharmit commented Jul 30, 2019

amitkrout commented Jul 30, 2019

amitkrout commented Jul 30, 2019 •

edited

amitkrout left a comment •

edited

surajnarwade left a comment

cdrage commented Jul 30, 2019

cdrage commented Jul 30, 2019

Make --output a global parameter, disable logging, fix bug #1898

Make --output a global parameter, disable logging, fix bug #1898

Conversation

cdrage commented Jul 10, 2019

cdrage commented Jul 10, 2019

cdrage commented Jul 12, 2019

cdrage commented Jul 16, 2019

amitkrout commented Jul 17, 2019

mik-dass commented Jul 17, 2019

mik-dass Jul 17, 2019 • edited

Choose a reason for hiding this comment

cdrage Jul 17, 2019

Choose a reason for hiding this comment

cdrage commented Jul 17, 2019

cdrage commented Jul 17, 2019

cdrage Jul 17, 2019

Choose a reason for hiding this comment

mik-dass commented Jul 18, 2019

cdrage commented Jul 18, 2019

mik-dass commented Jul 19, 2019

mik-dass commented Jul 26, 2019

mik-dass left a comment

Choose a reason for hiding this comment

openshift-ci-robot commented Jul 26, 2019

cdrage commented Jul 26, 2019

cdrage commented Jul 29, 2019

mohammedzee1000 left a comment

Choose a reason for hiding this comment

dharmit commented Jul 30, 2019

amitkrout commented Jul 30, 2019

amitkrout commented Jul 30, 2019 • edited

amitkrout left a comment • edited

Choose a reason for hiding this comment

surajnarwade left a comment

Choose a reason for hiding this comment

cdrage commented Jul 30, 2019

cdrage commented Jul 30, 2019

mik-dass Jul 17, 2019 •

edited

amitkrout commented Jul 30, 2019 •

edited

amitkrout left a comment •

edited