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

Do not call LogErrorAndExit but return err in GenericRun #6352

Merged

Conversation

feloy
Copy link
Contributor

@feloy feloy commented Nov 28, 2022

What type of PR is this:

/kind code-refactoring

What does this PR do / why we need it:

Which issue(s) this PR fixes:

Fixes #

PR acceptance criteria:

  • Unit test

  • Integration test

  • Documentation

How to test changes / Special notes to the reviewer:

@netlify
Copy link

netlify bot commented Nov 28, 2022

Deploy Preview for odo-docusaurus-preview canceled.

Name Link
🔨 Latest commit 4833b8c
🔍 Latest deploy log https://app.netlify.com/sites/odo-docusaurus-preview/deploys/63870b7b06c45d0009a17146

@odo-robot
Copy link

odo-robot bot commented Nov 28, 2022

Windows Tests (OCP) on commit 63fc615 finished successfully.
View logs: TXT HTML

@odo-robot
Copy link

odo-robot bot commented Nov 28, 2022

NoCluster Tests on commit 63fc615 finished successfully.
View logs: TXT HTML

@odo-robot
Copy link

odo-robot bot commented Nov 28, 2022

Unit Tests on commit 63fc615 finished successfully.
View logs: TXT HTML

@odo-robot
Copy link

odo-robot bot commented Nov 28, 2022

Validate Tests on commit 63fc615 finished successfully.
View logs: TXT HTML

@odo-robot
Copy link

odo-robot bot commented Nov 28, 2022

OpenShift Tests on commit 63fc615 finished successfully.
View logs: TXT HTML

@rm3l rm3l added this to the v3.4.0 🚀 milestone Nov 28, 2022
@odo-robot
Copy link

odo-robot bot commented Nov 28, 2022

Kubernetes Tests on commit 63fc615 finished successfully.
View logs: TXT HTML

@feloy feloy closed this Nov 28, 2022
@feloy feloy reopened this Nov 28, 2022
@feloy feloy closed this Nov 28, 2022
@feloy feloy reopened this Nov 28, 2022
Copy link
Member

@rm3l rm3l left a comment

Choose a reason for hiding this comment

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

I think there might be some other places to change.
Didn't check if that was also happening on main, but if I run odo with an unknown flag (say odo --xyz), Usage is displayed twice.

odo --xyz
$ odo --xyz

flag provided but not defined: -xyz
  __
 /  \__     odo is a CLI tool for fast iterative application development
 \__/  \    deployed immediately to your kubernetes cluster.
 /  \__/    Find more information at https://odo.dev
 \__/

Usage:
  odo [flags]
  odo [command]

Examples:
  Initializing your component by taking your pick from multiple languages or frameworks:
  odo init
  
  After creating your initial component, start development with:
  odo dev
  
  Want to deploy after development? See it live with:
  odo deploy

Main Commands:
  build-images Build images
  deploy       Deploy components
  dev          Deploy component to development cluster
  init         Init bootstraps a new project
  logs         Show logs of all containers of the component
  registry     List all components from the Devfile registry

Management Commands:
  add          Add resources to devfile (binding)
  create       Perform create operation (namespace)
  delete       Delete resources (component, namespace)
  describe     Describe resource (binding, component)
  list         List all components in the current namespace (binding, component, namespace, services)
  remove       Remove resources from devfile (binding)
  set          Perform set operation (namespace)

OpenShift Commands:
  login        Login to cluster 
  logout       Logout of the cluster 

Utility Commands:
  analyze      Detect devfile to use based on files present in current directory
  completion   Add odo completion support to your development environment
  preference   Modifies preference settings (add, remove, set, unset, view)
  version      Print the client version information

Flags:
      --complete             Install completion for odo command
      --kubeconfig string    Paths to a kubeconfig. Only required if out-of-cluster.
  -o, --o string             Specify output format, supported format: json
      --uncomplete           Uninstall completion for odo command
  -v, --v Level              Number for the log level verbosity. Level varies from 0 to 9 (default 0).
      --var stringArray      Variable to override Devfile variable and variables in var-file
      --var-file string      File containing variables to override Devfile variables
      --vmodule moduleSpec   Comma-separated list of pattern=N settings for file-filtered logging
  -y, --y                    Don't prompt user for typing 'yes' when installing completion

Use "odo [command] --help" for more information about a command.
Error: unknown flag: --xyz
Usage:
  odo [flags]
  odo [command]

Examples:
  Initializing your component by taking your pick from multiple languages or frameworks:
  odo init
  
  After creating your initial component, start development with:
  odo dev
  
  Want to deploy after development? See it live with:
  odo deploy

Main Commands:
  build-images Build images
  deploy       Deploy components
  dev          Deploy component to development cluster
  init         Init bootstraps a new project
  logs         Show logs of all containers of the component
  registry     List all components from the Devfile registry

Management Commands:
  add          Add resources to devfile (binding)
  create       Perform create operation (namespace)
  delete       Delete resources (component, namespace)
  describe     Describe resource (binding, component)
  list         List all components in the current namespace (binding, component, namespace, services)
  remove       Remove resources from devfile (binding)
  set          Perform set operation (namespace)

OpenShift Commands:
  login        Login to cluster 
  logout       Logout of the cluster 

Utility Commands:
  analyze      Detect devfile to use based on files present in current directory
  completion   Add odo completion support to your development environment
  preference   Modifies preference settings (add, remove, set, unset, view)
  version      Print the client version information

Flags:
      --complete             Install completion for odo command
  -h, --help                 Help for odo
      --kubeconfig string    Paths to a kubeconfig. Only required if out-of-cluster.
  -o, --o string             Specify output format, supported format: json
      --uncomplete           Uninstall completion for odo command
  -v, --v Level              Number for the log level verbosity. Level varies from 0 to 9 (default 0).
      --var stringArray      Variable to override Devfile variable and variables in var-file
      --var-file string      File containing variables to override Devfile variables
      --vmodule moduleSpec   Comma-separated list of pattern=N settings for file-filtered logging
  -y, --y                    Don't prompt user for typing 'yes' when installing completion

Use "odo [command] --help" for more information about a command.

 ✗  unknown flag: --xyz

@rm3l
Copy link
Member

rm3l commented Nov 28, 2022

I think there might be some other places to change. Didn't check if that was also happening on main, but if I run odo with an unknown flag (say odo --xyz), Usage is displayed twice.

odo --xyz

I just tested and noticed the same behavior on main, so this might be a different issue, not directly related to the changes here.

Copy link
Member

@rm3l rm3l left a comment

Choose a reason for hiding this comment

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

One slight thing I noticed on the behavior: when checking if ODO_DISABLE_TELEMETRY and ODO_TRACKING_CONSENT conflict, it looks like Usage is also being displayed:

$ ODO_DISABLE_TELEMETRY=true ODO_TRACKING_CONSENT=yes odo version
Error: ODO_DISABLE_TELEMETRY and ODO_TRACKING_CONSENT values are in conflict. ODO_DISABLE_TELEMETRY is deprecated, please use only ODO_TRACKING_CONSENT
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:
      --kubeconfig string    Paths to a kubeconfig. Only required if out-of-cluster.
  -o, --o string             Specify output format, supported format: json
  -v, --v Level              Number for the log level verbosity. Level varies from 0 to 9 (default 0).
      --var stringArray      Variable to override Devfile variable and variables in var-file
      --var-file string      File containing variables to override Devfile variables
      --vmodule moduleSpec   Comma-separated list of pattern=N settings for file-filtered logging

 ✗  ODO_DISABLE_TELEMETRY and ODO_TRACKING_CONSENT values are in conflict. ODO_DISABLE_TELEMETRY is deprecated, please use only ODO_TRACKING_CONSENT

And on main, I only get this, which makes more sense to me:

$ ODO_DISABLE_TELEMETRY=true ODO_TRACKING_CONSENT=yes odo version
 ✗  ODO_DISABLE_TELEMETRY and ODO_TRACKING_CONSENT values are in conflict. ODO_DISABLE_TELEMETRY is deprecated, please use only ODO_TRACKING_CONSENT

pkg/odo/genericclioptions/runnable.go Show resolved Hide resolved
pkg/odo/genericclioptions/runnable.go Show resolved Hide resolved
pkg/odo/genericclioptions/runnable.go Show resolved Hide resolved
pkg/odo/genericclioptions/runnable.go Show resolved Hide resolved
pkg/odo/genericclioptions/runnable.go Show resolved Hide resolved
@sonarcloud
Copy link

sonarcloud bot commented Nov 30, 2022

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 1 Code Smell

No Coverage information No Coverage information
0.0% 0.0% Duplication

@feloy feloy requested a review from rm3l November 30, 2022 07:54
Copy link
Member

@rm3l rm3l left a comment

Choose a reason for hiding this comment

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

/approve

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. Required by Prow. label Nov 30, 2022
@openshift-ci
Copy link

openshift-ci bot commented Nov 30, 2022

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: rm3l

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 openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. Required by Prow. label Nov 30, 2022
@feloy
Copy link
Contributor Author

feloy commented Nov 30, 2022

/override ci/prow/v4.11-integration-e2e
Flaky e2e tests failing

@openshift-ci
Copy link

openshift-ci bot commented Nov 30, 2022

@feloy: Overrode contexts on behalf of feloy: ci/prow/v4.11-integration-e2e

In response to this:

/override ci/prow/v4.11-integration-e2e
Flaky e2e tests failing

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@openshift-merge-robot openshift-merge-robot merged commit 3fc9f97 into redhat-developer:main Nov 30, 2022
@rm3l rm3l added the area/refactoring Issues or PRs related to code refactoring label Jun 19, 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. area/refactoring Issues or PRs related to code refactoring 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.

3 participants