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.

Sign up for GitHub

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

Jump to bottom

Fix install/upgrade chart error notifications #4729

Merged
merged 2 commits into from
Nov 3, 2020
Merged

Fix install/upgrade chart error notifications #4729

merged 2 commits into from
Nov 3, 2020

Conversation

richard-cox
Copy link
Contributor

fixes #4673

  • errors that are 5XX show up as endpoint errors, these are tied into the notification bell and are distruptive
  • if there's something wrong in the chart we often got a 5XX error
  • in src/jetstream/plugins/kubernetes/install_release.go it's hard to determine if the error is user or system based
  • this pr switches the assumption to all user based errors

fixes #4672

  • the error message associated with a helm install can be pretty big
  • ensure stepper snack bar can handle it

@richard-cox richard-cox self-assigned this Oct 30, 2020
@codecov-io
Copy link

codecov-io commented Oct 30, 2020 •
edited
Loading

Codecov Report

Merging #4729 into master will decrease coverage by 0.00%.
The diff coverage is 0.00%.

@@            Coverage Diff             @@
##           master    #4729      +/-   ##
==========================================
- Coverage   54.44%   54.44%   -0.01%     
==========================================
  Files        1120     1120              
  Lines       38961    38962       +1     
  Branches     4904     4904              
==========================================
  Hits        21211    21211              
- Misses      17574    17575       +1     
  Partials      176      176

richard-cox
richard-cox commented Nov 2, 2020
View reviewed changes
src/jetstream/plugins/kubernetes/install_release.go
}

// Client must give us the download URL for the chart
if len(params.ChartURL) == 0 {
return interfaces.NewJetstreamErrorf("Client did not supply Chart download URL")
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Stays as NewJetstreamErrorf

@richard-cox richard-cox added the needs attention This PR needs attention label Nov 2, 2020
@richard-cox richard-cox added comments-addressed and removed needs attention This PR needs attention labels Nov 2, 2020
nwmac
nwmac approved these changes Nov 3, 2020
View reviewed changes
Copy link
Contributor

@nwmac nwmac left a comment

Choose a reason for hiding this comment

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

LGTM

@richard-cox richard-cox merged commit 8ed7bbe into master Nov 3, 2020
@richard-cox richard-cox deleted the fix-helm-failure-messages branch November 3, 2020 10:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nwmac nwmac nwmac approved these changes

Assignees

@richard-cox richard-cox

Labels
comments-addressed ready for review
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Helm Install: Errors can incorrectly show up as endpoint errors Helm Install: Trim error message in snackbar
3 participants
@richard-cox @codecov-io @nwmac