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

Output usage only when flag parsing fails #4381

Closed
ffjlabo opened this issue Jun 8, 2023 · 7 comments · Fixed by #4464
Closed

Output usage only when flag parsing fails #4381

ffjlabo opened this issue Jun 8, 2023 · 7 comments · Fixed by #4464
Assignees
Labels
area/pipectl kind/enhancement New feature or request

Comments

@ffjlabo
Copy link
Member

ffjlabo commented Jun 8, 2023

What would you like to be added:

I propose that clis output usage only when flag parsing fails, not when internal errors occur.

Why is this needed:

When I tried to do quickstart with pipectl, an error occurs.

I tried to do quickstart with pipectl and encounter the error log below
At first glance, it seems that only the stack trace is displayed, but in fact the specific error message is displayed at the bottom.
In my opinion, it is difficult to find the message quickly.

% pipectl quickstart --version v0.43.1                                                                     (git)-[master]
Installing the controlplane in quickstart mode...
Failed to install PipeCD control plane!!
github.com/pipe-cd/pipecd/pkg/app/pipectl/cmd/quickstart.(*command).run
	/home/runner/work/pipecd/pipecd/pkg/app/pipectl/cmd/quickstart/quickstart.go:114
github.com/pipe-cd/pipecd/pkg/cli.runWithContext
	/home/runner/work/pipecd/pipecd/pkg/cli/cmd.go:95
github.com/pipe-cd/pipecd/pkg/cli.WithContext.func1
	/home/runner/work/pipecd/pipecd/pkg/cli/cmd.go:51
github.com/spf13/cobra.(*Command).execute
	/home/runner/go/pkg/mod/github.com/spf13/cobra@v1.0.0/command.go:842
github.com/spf13/cobra.(*Command).ExecuteC
	/home/runner/go/pkg/mod/github.com/spf13/cobra@v1.0.0/command.go:950
github.com/spf13/cobra.(*Command).Execute
	/home/runner/go/pkg/mod/github.com/spf13/cobra@v1.0.0/command.go:887
github.com/pipe-cd/pipecd/pkg/cli.(*App).Run
	/home/runner/work/pipecd/pipecd/pkg/cli/app.go:60
main.main
	/home/runner/work/pipecd/pipecd/cmd/pipectl/main.go:47
runtime.main
	/opt/hostedtoolcache/go/1.19.3/x64/src/runtime/proc.go:250
Usage:
  pipectl quickstart [flags]

Flags:
  -h, --help               help for quickstart
      --namespace string   The Kubernetes cluster namespace where to install Control Plane. (default "pipecd")
      --tools-dir string   The path to directory where to install tools such as helm. (default "/Users/s14218/.pipectl/tools")
      --uninstall          Uninstall the quickstart mode installed PipeCD control plane.
      --version string     The Control Plane version. Default is the version of pipectl. (default "v0.43.1")

Global Flags:
      --log-encoding string                The encoding type for logger [json|console|humanize]. (default "humanize")
      --log-level string                   The minimum enabled logging level. (default "info")
      --metrics                            Whether metrics is enabled or not. (default true)
      --profile                            If true enables uploading the profiles to Stackdriver.
      --profile-debug-logging              If true enables logging debug information of profiler.
      --profiler-credentials-file string   The path to the credentials file using while sending profiles to Stackdriver.

Error: exit status 1: Error: Kubernetes cluster unreachable: Get "https://127.0.0.1:50668/version": dial tcp 127.0.0.1:50668: connect: connection refused

This is because usage is shown before cli handles error on app.Run().
By default, cobra shows usage when command.Execute(). ref: the internal of it

@ffjlabo ffjlabo added the kind/enhancement New feature or request label Jun 8, 2023
@ffjlabo
Copy link
Member Author

ffjlabo commented Jun 8, 2023

In this comment on the cobra's issue, it is suggested to disable showing usage by default, and show it only when flag parsing fails. So how would you like to fix like this?

ffjlabo added a commit to ffjlabo/pipecd that referenced this issue Jun 8, 2023
@ffjlabo
Copy link
Member Author

ffjlabo commented Jun 8, 2023

FYI, this is draft fix ffjlabo@d395ee3.

when parsing fails

% .artifacts/pipectl quickstart --versio v0.43.1                                (git)-[show-usage-only-when-parsing-err]
unknown flag: --versio
Usage:
  pipectl quickstart [flags]

Flags:
  -h, --help               help for quickstart
      --namespace string   The Kubernetes cluster namespace where to install Control Plane. (default "pipecd")
      --tools-dir string   The path to directory where to install tools such as helm. (default "/Users/s14218/.pipectl/tools")
      --uninstall          Uninstall the quickstart mode installed PipeCD control plane.
      --version string     The Control Plane version. Default is the version of pipectl. (default "v0.25.1-870-gd395ee3")

Global Flags:
      --log-encoding string                The encoding type for logger [json|console|humanize]. (default "humanize")
      --log-level string                   The minimum enabled logging level. (default "info")
      --metrics                            Whether metrics is enabled or not. (default true)
      --profile                            If true enables uploading the profiles to Stackdriver.
      --profile-debug-logging              If true enables logging debug information of profiler.
      --profiler-credentials-file string   The path to the credentials file using while sending profiles to Stackdriver.

Error: FlagParseErr

when internal errors

% .artifacts/pipectl quickstart --version v0.43.1                               (git)-[show-usage-only-when-parsing-err]
Installing the controlplane in quickstart mode...
Failed to install PipeCD control plane!!
github.com/pipe-cd/pipecd/pkg/app/pipectl/cmd/quickstart.(*command).run
	/Users/s14218/works/ffjlabo/pipecd/pkg/app/pipectl/cmd/quickstart/quickstart.go:114
github.com/pipe-cd/pipecd/pkg/cli.runWithContext
	/Users/s14218/works/ffjlabo/pipecd/pkg/cli/cmd.go:95
github.com/pipe-cd/pipecd/pkg/cli.WithContext.func1
	/Users/s14218/works/ffjlabo/pipecd/pkg/cli/cmd.go:51
github.com/spf13/cobra.(*Command).execute
	/Users/s14218/go/pkg/mod/github.com/spf13/cobra@v1.0.0/command.go:842
github.com/spf13/cobra.(*Command).ExecuteC
	/Users/s14218/go/pkg/mod/github.com/spf13/cobra@v1.0.0/command.go:950
github.com/spf13/cobra.(*Command).Execute
	/Users/s14218/go/pkg/mod/github.com/spf13/cobra@v1.0.0/command.go:887
github.com/pipe-cd/pipecd/pkg/cli.(*App).Run
	/Users/s14218/works/ffjlabo/pipecd/pkg/cli/app.go:69
main.main
	/Users/s14218/works/ffjlabo/pipecd/cmd/pipectl/main.go:47
runtime.main
	/opt/homebrew/Cellar/go/1.20.3/libexec/src/runtime/proc.go:250
Error: exit status 1: Error: Kubernetes cluster unreachable: Get "https://127.0.0.1:50668/version": dial tcp 127.0.0.1:50668: connect: connection refused

@khanhtc1202
Copy link
Member

Nice, could you commit the fix 🙏 And welcome back @ffjlabo 🙌

@ffjlabo
Copy link
Member Author

ffjlabo commented Jun 10, 2023

It's been a while @khanhtc1202 san!
Thank you! I'll make a PR 👍

Oh...I forgot to tell one thing.
Is it OK that fix affects not only pipectl but also any other clis using cli.NewApp?

@khanhtc1202
Copy link
Member

@ffjlabo For sure if you think it's the better choice 😄

@khanhtc1202
Copy link
Member

@ffjlabo Hi there, any update on this? If possible, we would like to add this to the next release (v0.44.2) Please lets me know if you want some help 😄

@ffjlabo
Copy link
Member Author

ffjlabo commented Jul 5, 2023

@khanhtc1202 Sorry to be late. I forgot to make a PR 🙇
Soon I make it!

ffjlabo added a commit to ffjlabo/pipecd that referenced this issue Jul 5, 2023
Signed-off-by: Yoshiki Fujikane <ffjlabo@gmail.com>
ffjlabo added a commit to ffjlabo/pipecd that referenced this issue Jul 5, 2023
Signed-off-by: Yoshiki Fujikane <ffjlabo@gmail.com>
khanhtc1202 pushed a commit that referenced this issue Jul 5, 2023
Signed-off-by: Yoshiki Fujikane <ffjlabo@gmail.com>
kentakozuka pushed a commit that referenced this issue Jul 12, 2023
Signed-off-by: Yoshiki Fujikane <ffjlabo@gmail.com>
kentakozuka pushed a commit that referenced this issue Jul 12, 2023
Signed-off-by: Yoshiki Fujikane <ffjlabo@gmail.com>
Signed-off-by: Kenta Kozuka <kenta.kozuka@gmail.com>
kentakozuka added a commit that referenced this issue Jul 12, 2023
)

* Update web deps (#4451)

Signed-off-by: khanhtc1202 <khanhtc1202@gmail.com>
Signed-off-by: Kenta Kozuka <kenta.kozuka@gmail.com>

* Update quickstart to support arm hardware(ex: M1) (#4457)

* Add: arch selection for mysql image
Update chart values and templates for mysql deployment

Signed-off-by: gitbluf <gitbluf@tuta.io>

* Update: MySql image selector function to be more descriptive

Signed-off-by: gitbluf <gitbluf@tuta.io>

* Remove: mysql image set on piped installation

Signed-off-by: gitbluf <gitbluf@tuta.io>

* Update method name

Signed-off-by: gitbluf <gitbluf@tuta.io>

---------

Signed-off-by: gitbluf <gitbluf@tuta.io>
Signed-off-by: Kenta Kozuka <kenta.kozuka@gmail.com>

* Support AWS LoadBalancer with multi listeners (#4462)

* Support AWS LoadBalancer with multi listeners

Signed-off-by: khanhtc1202 <khanhtc1202@gmail.com>

* Fix go array index

Signed-off-by: khanhtc1202 <khanhtc1202@gmail.com>

* Rename symbol

Signed-off-by: khanhtc1202 <khanhtc1202@gmail.com>

---------

Signed-off-by: khanhtc1202 <khanhtc1202@gmail.com>
Signed-off-by: Kenta Kozuka <kenta.kozuka@gmail.com>

* Specify Version For Templating Tools (#4463)

* specify version for templating tools

Signed-off-by: Viet Anh Pham Nhu <nhuvietanh271098@gmail.com>

* Handle Error Both Install Helm And Kustomize

Signed-off-by: Viet Anh Pham Nhu <nhuvietanh271098@gmail.com>

---------

Signed-off-by: Viet Anh Pham Nhu <nhuvietanh271098@gmail.com>
Signed-off-by: Kenta Kozuka <kenta.kozuka@gmail.com>

* Output usage only when flag parsing fails #4381 (#4464)

Signed-off-by: Yoshiki Fujikane <ffjlabo@gmail.com>
Signed-off-by: Kenta Kozuka <kenta.kozuka@gmail.com>

* Fix ECS rollback stage does not remove canary created tasks (#4465)

* Fix cannot remove canary created task

Signed-off-by: ductnn <trannhuduc531998@gmail.com>

* Fix reuse clean function

Signed-off-by: ductnn <trannhuduc531998@gmail.com>

* Update logs message for ECS routing stage rollback

Signed-off-by: ductnn <trannhuduc531998@gmail.com>

---------

Signed-off-by: ductnn <trannhuduc531998@gmail.com>
Signed-off-by: Kenta Kozuka <kenta.kozuka@gmail.com>

* Update logs message for ECS routing stage executor (#4466)

Signed-off-by: khanhtc1202 <khanhtc1202@gmail.com>
Signed-off-by: Kenta Kozuka <kenta.kozuka@gmail.com>

* fix: update deprecated apiVersion since gke removed (#4469)

GoogleCloudPlatform/gke-managed-certs#58

Signed-off-by: hungran <26101787+hungran@users.noreply.github.com>
Signed-off-by: Kenta Kozuka <kenta.kozuka@gmail.com>

* Skip update AppState when no log updates (#4482)

Signed-off-by: Kenta Kozuka <kenta.kozuka@gmail.com>

---------

Signed-off-by: khanhtc1202 <khanhtc1202@gmail.com>
Signed-off-by: Kenta Kozuka <kenta.kozuka@gmail.com>
Signed-off-by: gitbluf <gitbluf@tuta.io>
Signed-off-by: Viet Anh Pham Nhu <nhuvietanh271098@gmail.com>
Signed-off-by: Yoshiki Fujikane <ffjlabo@gmail.com>
Signed-off-by: ductnn <trannhuduc531998@gmail.com>
Signed-off-by: hungran <26101787+hungran@users.noreply.github.com>
Co-authored-by: Khanh Tran <32532742+khanhtc1202@users.noreply.github.com>
Co-authored-by: Marko Petrovic <22802784+gitbluf@users.noreply.github.com>
Co-authored-by: Viet Anh Pham Nhu <40441000+anhpnv@users.noreply.github.com>
Co-authored-by: Yoshiki Fujikane <40124947+ffjlabo@users.noreply.github.com>
Co-authored-by: Duc Tran <trannhuduc531998@gmail.com>
Co-authored-by: Henry Vu <26101787+hungran@users.noreply.github.com>
Co-authored-by: Kurochan <kuro@kurochan.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/pipectl kind/enhancement New feature or request
Projects
No open projects
Status: Done
Development

Successfully merging a pull request may close this issue.

2 participants