Errors out if json machine output has not been implemented #2160

cdrage · 2019-09-23T20:13:33Z

This PR:

Moves annotations to &cobra.Command struct creation rather than
using the command to add it
Errors out if -o json has been passed, but json support has not
been added
Hides the -o json command if it's not impemented
Solves the issue of Remove -o, --o string from global flag #2031
Changes "Global Flags" to "Additional Flags"

Closes #2031

cdrage · 2019-09-23T20:14:29Z

@amitkrout if you could please review that'd be great.

Keep in mind that this does not remove -o from the help menu, but rather, it will error out if the user passes in -o json to a command that does not have the functionality implemented.

mik-dass · 2019-09-24T04:39:44Z

odo storage list --context /tmp/800490314 -o json]
[odo]  ✗  Machine readable output is not yet implemented for this command

@cdrage storage list is missing the required changes

mik-dass · 2019-09-25T07:35:01Z

@cdrage seems like odo version also needs the changes too

odo sub component command tests Generic machine readable output tests 
  Command should fail if json is non-existent for a command<`2`>
  /home/travis/gopath/src/github.com/openshift/odo/tests/integration/component.go:44
Created dir: /tmp/602711673
Running odo with args [odo version -o json]
[odo]  ✗  Machine readable output is not yet implemented for this command
Deleting dir: /tmp/602711673

mik-dass · 2019-09-25T07:36:54Z

tests/integration/component.go

+			// `odo version` does not have json implemented, and thus we will use this
+			// as a test
+			output := helper.CmdShouldFail("odo", "version", "-o", "json")
+			Expect(output).To(ContainSubstring("Machine readable output is not implemented for this command"))


Expected <string>: ✗ Machine readable output is not yet implemented for this command to contain substring <string>: Machine readable output is not implemented for this command

is not yet implemented

cdrage · 2019-09-25T15:56:51Z

/test v4.1-integration

mik-dass · 2019-09-26T08:10:15Z

 <*os.PathError | 0xc0003060c0>: {Op: "chdir", Path: "", Err: 0x2}
        chdir : no such file or directory

/retest

mik-dass · 2019-09-30T07:56:34Z

[odo]  ✗  namespaces "mhimfxghro" already exists
[odo]  ✗  Waiting for project to come up [68ms]
Deleting project: 
Running odo with args

/retest

mohammedzee1000

Looking good
/approve

mohammedzee1000 · 2019-10-01T11:58:22Z

pkg/odo/cli/storage/list.go

+		Long:        storageListLongDesc,
+		Example:     fmt.Sprintf(storageListExample, fullName),
+		Args:        cobra.MaximumNArgs(1),
+		Annotations: map[string]string{"machineoutput": "json"},


This is mostly for all of them, interesting use of annotations :)

mohammedzee1000 · 2019-10-01T12:00:54Z

pkg/odo/genericclioptions/runnable.go

+	flagChanged := outputFlag != nil && outputFlag.Changed
+	machineOutput := cmd.Annotations["machineoutput"]
+
+	if flagChanged && outputFlag.Value.String() != "json" {
 		log.Error("Please input a valid output format for -o, available format: json")


All these (log.Error + exit 1) could possibly be replaced by alog.Fatal like the standard log package maybe. Anycase more of an implementation detail :)

Yeah, we use log.Error instead since we're able to modify the output to something more pretty :)

openshift-ci-robot · 2019-10-01T12:01:47Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: mohammedzee1000

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/odo/OWNERS~~ [mohammedzee1000]
~~tests/OWNERS~~ [mohammedzee1000]

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

kadel · 2019-10-02T17:11:23Z

I can still see that the help messages are showing the -o flag even for commands where it is not implemented.
@cdrage I taught that you mentioned that we can hide it from the output, so it is not shown for the commands where it is not implemented.

cdrage · 2019-10-02T17:59:23Z

@kadel Yes, this is only one half of the issue. This errors out if it's been passed in, but I have not yet updated it to hide the actual flag.. I'll set this as WIP for now.

kadel · 2019-10-03T07:25:50Z

@kadel Yes, this is only one half of the issue. This errors out if it's been passed in, but I have not yet updated it to hide the actual flag.. I'll set this as WIP for now.

We can split it into multiple PRs if you want. I just wanted to make sure that we forgot this part, as it is also really important.

cdrage · 2019-10-08T20:07:19Z

Updated the PR! Ready for review @kadel and @girishramnani

I've updated the PR's description and implementing hiding the command.

Global Flags has also been renamed to Additional Flags.

See below for the changes:

github.com/openshift/odo  remove-json-from-some-commands ✔                                                                                                                                                                                                                 13m  ⍉
▶ ./odo version --help
Print the client version information

Usage:
  odo version [flags]

Examples:
  # Print the client version of odo
  odo version

Flags:
      --client   Client version only (no server required).
  -h, --help     Help for version

Additional Flags:
  -v, --v Level              Log level for V logs. Level varies from 0 to 9 (default 0).
      --vmodule moduleSpec   Comma-separated list of pattern=N settings for file-filtered logging

▶ ./odo service list --help        
List all services in the current application

Usage:
  odo service list [flags]

Examples:
  # List all services in the application
  odo service list

Flags:
      --app string       Application, defaults to active application
      --context string   Use given context directory as a source for component settings
  -h, --help             Help for list
      --project string   Project, defaults to active project

Additional Flags:
  -o, --o string             Specify output format, supported format: json (default "json")
  -v, --v Level              Log level for V logs. Level varies from 0 to 9 (default 0).
      --vmodule moduleSpec   Comma-separated list of pattern=N settings for file-filtered logging

cdrage · 2019-10-17T19:24:13Z

/retest

mik-dass · 2019-10-18T06:26:08Z

'error: unable to connect to a server to handle {"image.openshift.io" "v1" "imagestreams"}: invalid configuration: Missing or incomplete configuration info.  Please login or point to an existing, complete config file:
[oc] 
[oc]   1. Via the command-line flag --config
[oc]   2. Via the KUBECONFIG environment variable
[oc]   3. In your home directory as ~/.kube/config

/retest

mik-dass · 2019-10-18T06:44:33Z

pkg/odo/cli/component/create.go

@@ -593,7 +594,6 @@ func NewCmdCreate(name, fullName string) *cobra.Command {
 	componentCreateCmd.Flags().StringSliceVarP(&co.componentPorts, "port", "p", []string{}, "Ports to be used when the component is created (ex. 8080,8100/tcp,9100/udp)")
 	componentCreateCmd.Flags().StringSliceVar(&co.componentEnvVars, "env", []string{}, "Environmental variables for the component. For example --env VariableName=Value")
 	// Add a defined annotation in order to appear in the help menu


please remove this comment

mik-dass · 2019-10-18T06:44:46Z

pkg/odo/cli/component/delete.go

@@ -133,7 +134,6 @@ func NewCmdDelete(name, fullName string) *cobra.Command {
 	componentDeleteCmd.Flags().BoolVarP(&do.componentDeleteAllFlag, "all", "a", false, "Delete component and local config")

 	// Add a defined annotation in order to appear in the help menu


please remove this comment

mik-dass · 2019-10-18T06:45:00Z

pkg/odo/cli/component/describe.go

+		Long:        `Describe component.`,
+		Example:     fmt.Sprintf(describeExample, fullName),
+		Args:        cobra.RangeArgs(0, 1),
+		Annotations: map[string]string{"machineoutput": "json", "command": "component"},
 		Run: func(cmd *cobra.Command, args []string) {
 			genericclioptions.GenericRun(do, cmd, args)
 		},
 	}

 	// Add a defined annotation in order to appear in the help menu


same as above

mik-dass · 2019-10-18T06:45:08Z

pkg/odo/cli/component/link.go

@@ -134,7 +135,6 @@ func NewCmdLink(name, fullName string) *cobra.Command {
 	linkCmd.PersistentFlags().BoolVar(&o.waitForTarget, "wait-for-target", false, "If enabled, the link command will wait for the service to be provisioned (has no effect when linking to a component)")

 	// Add a defined annotation in order to appear in the help menu


same as above

mik-dass · 2019-10-18T06:45:16Z

pkg/odo/cli/component/list.go

+		Long:        "List all components in the current application.",
+		Example:     fmt.Sprintf(listExample, fullName),
+		Args:        cobra.NoArgs,
+		Annotations: map[string]string{"machineoutput": "json", "command": "component"},
 		Run: func(cmd *cobra.Command, args []string) {
 			genericclioptions.GenericRun(o, cmd, args)
 		},
 	}
 	// Add a defined annotation in order to appear in the help menu


same as above

mik-dass · 2019-10-18T06:45:27Z

pkg/odo/cli/component/log.go

@@ -74,7 +75,6 @@ func NewCmdLog(name, fullName string) *cobra.Command {
 	logCmd.Flags().BoolVarP(&o.logFollow, "follow", "f", false, "Follow logs")

 	// Add a defined annotation in order to appear in the help menu


same as above

mik-dass · 2019-10-18T06:45:36Z

pkg/odo/cli/component/push.go

@@ -127,7 +128,6 @@ func NewCmdPush(name, fullName string) *cobra.Command {
 	pushCmd.Flags().BoolVarP(&po.forceBuild, "force-build", "f", false, "Use force-build flag to force building the component")

 	// Add a defined annotation in order to appear in the help menu


same as above

mik-dass · 2019-10-18T06:45:43Z

pkg/odo/cli/component/unlink.go

@@ -87,7 +88,6 @@ func NewCmdUnlink(name, fullName string) *cobra.Command {
 	unlinkCmd.PersistentFlags().BoolVarP(&o.wait, "wait", "w", false, "If enabled the link will return only when the component is fully running after the link is deleted")

 	// Add a defined annotation in order to appear in the help menu


same as above

mik-dass · 2019-10-18T06:45:52Z

pkg/odo/cli/component/update.go

@@ -167,7 +168,6 @@ func NewCmdUpdate(name, fullName string) *cobra.Command {
 	updateCmd.Flags().StringVarP(&uo.local, "local", "l", "", "Use local directory as a source for component.")
 	updateCmd.Flags().StringVarP(&uo.ref, "ref", "r", "", "Use a specific ref e.g. commit, branch or tag of the git repository")
 	// Add a defined annotation in order to appear in the help menu


same as above

mik-dass · 2019-10-18T06:46:02Z

pkg/odo/cli/component/watch.go

@@ -162,7 +163,6 @@ func NewCmdWatch(name, fullName string) *cobra.Command {
 	watchCmd.Flags().IntVar(&wo.delay, "delay", 1, "Time in seconds between a detection of code change and push.delay=0 means changes will be pushed as soon as they are detected which can cause performance issues")

 	// Add a defined annotation in order to appear in the help menu


same as above

cdrage · 2019-10-18T14:02:09Z

Thanks @mik-dass for the review! updated the PR again.

cdrage · 2019-10-18T14:58:37Z

/retest

mik-dass · 2019-10-21T07:52:40Z

agged from registry.centos.org/dotnet/dotnet-20-centos7
[oc] 
[oc]   ! error: Import failed (InternalError): Internal error occurred: Get https://registry.centos.org/v2/: net/http: request canceled while waiting for connection (Client.Timeout exceeded while awaiting headers)
[oc]       Less than a second ago
[oc]

/retest

girishramnani · 2019-10-21T09:00:28Z

reviewed the PR just waiting to confirm if all the comments from @mik-dass were resolved

mik-dass

/lgtm

openshift-bot · 2019-10-21T13:59:48Z

/retest

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

cdrage · 2019-11-07T16:08:20Z

/kind bug

openshift-ci-robot added the size/L label Sep 23, 2019

openshift-ci-robot requested review from girishramnani and mohammedzee1000 September 23, 2019 20:13

cdrage force-pushed the remove-json-from-some-commands branch from b45a29e to f1fc55a Compare September 24, 2019 17:32

mik-dass reviewed Sep 25, 2019

View reviewed changes

cdrage force-pushed the remove-json-from-some-commands branch from f1fc55a to eff9b35 Compare September 25, 2019 12:10

cdrage force-pushed the remove-json-from-some-commands branch from eff9b35 to f3953f4 Compare September 25, 2019 19:39

cdrage force-pushed the remove-json-from-some-commands branch from f3953f4 to a82cb0e Compare September 27, 2019 20:00

mohammedzee1000 approved these changes Oct 1, 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 Oct 1, 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 Oct 2, 2019

cdrage force-pushed the remove-json-from-some-commands branch 4 times, most recently from cbf1b4e to ac3cb5a Compare October 8, 2019 19:51

openshift-ci-robot 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 Oct 8, 2019

cdrage force-pushed the remove-json-from-some-commands branch from ac3cb5a to 68066ee Compare October 8, 2019 19:58

cdrage force-pushed the remove-json-from-some-commands branch from 68066ee to 7931acc Compare October 9, 2019 12:49

cdrage mentioned this pull request Oct 9, 2019

Flags naming and description issues in help message #1671

Closed

mik-dass reviewed Oct 18, 2019

View reviewed changes

Remove -o from global flags when doing --help

44a8775

cdrage force-pushed the remove-json-from-some-commands branch from baa7657 to 44a8775 Compare October 18, 2019 13:56

mik-dass approved these changes Oct 21, 2019

View reviewed changes

openshift-ci-robot assigned mik-dass Oct 21, 2019

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

openshift-merge-robot merged commit 1269395 into redhat-developer:master Oct 21, 2019

cdrage added the kind/enhancement label Nov 6, 2019

openshift-ci-robot added the kind/bug Categorizes issue or PR as related to a bug. label Nov 7, 2019

girishramnani mentioned this pull request Jan 2, 2020

Machine readable output - global -o flag #1475

Closed

cdrage deleted the remove-json-from-some-commands branch January 14, 2022 14:54

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

		@@ -133,7 +134,6 @@ func NewCmdDelete(name, fullName string) *cobra.Command {
		componentDeleteCmd.Flags().BoolVarP(&do.componentDeleteAllFlag, "all", "a", false, "Delete component and local config")

		// Add a defined annotation in order to appear in the help menu

		@@ -134,7 +135,6 @@ func NewCmdLink(name, fullName string) *cobra.Command {
		linkCmd.PersistentFlags().BoolVar(&o.waitForTarget, "wait-for-target", false, "If enabled, the link command will wait for the service to be provisioned (has no effect when linking to a component)")

		// Add a defined annotation in order to appear in the help menu

		@@ -74,7 +75,6 @@ func NewCmdLog(name, fullName string) *cobra.Command {
		logCmd.Flags().BoolVarP(&o.logFollow, "follow", "f", false, "Follow logs")

		// Add a defined annotation in order to appear in the help menu

		@@ -127,7 +128,6 @@ func NewCmdPush(name, fullName string) *cobra.Command {
		pushCmd.Flags().BoolVarP(&po.forceBuild, "force-build", "f", false, "Use force-build flag to force building the component")

		// Add a defined annotation in order to appear in the help menu

		@@ -87,7 +88,6 @@ func NewCmdUnlink(name, fullName string) *cobra.Command {
		unlinkCmd.PersistentFlags().BoolVarP(&o.wait, "wait", "w", false, "If enabled the link will return only when the component is fully running after the link is deleted")

		// Add a defined annotation in order to appear in the help menu

		@@ -162,7 +163,6 @@ func NewCmdWatch(name, fullName string) *cobra.Command {
		watchCmd.Flags().IntVar(&wo.delay, "delay", 1, "Time in seconds between a detection of code change and push.delay=0 means changes will be pushed as soon as they are detected which can cause performance issues")

		// Add a defined annotation in order to appear in the help menu

Errors out if json machine output has not been implemented #2160

Errors out if json machine output has not been implemented #2160

Conversation

cdrage commented Sep 23, 2019 • edited Loading

cdrage commented Sep 23, 2019

mik-dass commented Sep 24, 2019

mik-dass commented Sep 25, 2019

Choose a reason for hiding this comment

cdrage commented Sep 25, 2019

mik-dass commented Sep 26, 2019

mik-dass commented Sep 30, 2019

mohammedzee1000 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mohammedzee1000 Oct 1, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

openshift-ci-robot commented Oct 1, 2019

kadel commented Oct 2, 2019

cdrage commented Oct 2, 2019

kadel commented Oct 3, 2019

cdrage commented Oct 8, 2019

cdrage commented Oct 17, 2019

mik-dass commented Oct 18, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cdrage commented Oct 18, 2019

cdrage commented Oct 18, 2019

mik-dass commented Oct 21, 2019

girishramnani commented Oct 21, 2019

mik-dass left a comment

Choose a reason for hiding this comment

openshift-bot commented Oct 21, 2019

cdrage commented Nov 7, 2019

cdrage commented Sep 23, 2019 •

edited

Loading

mohammedzee1000 Oct 1, 2019 •

edited

Loading