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

chore: Improved error handling and refactor #2184

Merged
merged 4 commits into from
Feb 6, 2020

Conversation

simster7
Copy link
Member

@simster7 simster7 commented Feb 6, 2020

Closes #2165 by improving error handling to display the JSON response message when available, and not allow the submission of malformed yamls while displaying the reason they are malformed.

Also brings this functionality to CronWorkflow and WorkflowTemplate submission, which previously did not have it.

@simster7 simster7 requested a review from alexec February 6, 2020 21:22
@simster7 simster7 added this to the v2.5 milestone Feb 6, 2020
@alexec alexec self-assigned this Feb 6, 2020
Copy link
Contributor

@alexec alexec left a comment

Choose a reason for hiding this comment

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

Rubber-stamp approval. I've not checked this out or tested it - so only merge if you have high confidence in the changes. Can you make sure that the workflow template one works? I'm pretty sure this is broken on master.

@codecov
Copy link

codecov bot commented Feb 6, 2020

Codecov Report

Merging #2184 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #2184   +/-   ##
=======================================
  Coverage   11.71%   11.71%           
=======================================
  Files          51       51           
  Lines       26309    26309           
=======================================
  Hits         3081     3081           
  Misses      22833    22833           
  Partials      395      395

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update fc476b2...d69e6d1. Read the comment docs.

@simster7
Copy link
Member Author

simster7 commented Feb 6, 2020

Double checked everything works as intended.

Can you make sure that the workflow template one works? I'm pretty sure this is broken on master.

Yup, it was indeed broken. Fixed!

@simster7 simster7 merged commit 7824530 into argoproj:master Feb 6, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

When UI submission fails, errors are always "Internal Server Error"
2 participants