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

Surface vis loader errors in the UI. #30594

Merged
merged 3 commits into from
Feb 22, 2019
Merged

Surface vis loader errors in the UI. #30594

merged 3 commits into from
Feb 22, 2019

Conversation

lukeelmers
Copy link
Member

@lukeelmers lukeelmers commented Feb 8, 2019 •
edited
Loading

Closes #29900

Fixes issue where errors were not caught and displayed in the UI when using the pipeline data loader.

I pulled the error handling logic out into a function that can be shared between VisualizeDataLoader and PipelineDataLoader.

Notes for reviewers:

  • It is recommended to review the diff with "Hide whitespace changes" selected.
  • This was previously reviewed/approved in Share vis loader error handling. #30167, which was closed after a failed merge and replaced with this PR. Refer to discussion there for more detail.

@lukeelmers lukeelmers added v7.0.0 Feature:ExpressionLanguage Interpreter expression language (aka canvas pipeline) Team:Visualizations Visualization editors, elastic-charts and infrastructure v8.0.0 :pipeline-bugs labels Feb 8, 2019
@lukeelmers lukeelmers requested a review from timroes February 8, 2019 23:18
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-app

@lukeelmers lukeelmers mentioned this pull request Feb 8, 2019
Closed
@lukeelmers lukeelmers changed the title Handle vis loader errors. Surface vis loader errors in the UI. Feb 8, 2019
@elasticmachine

This comment has been minimized.

Sign in to view
@lukeelmers lukeelmers force-pushed the fix/vis-loader-errors branch from 5feb58c to 0f551b9 Compare February 11, 2019 21:07
@lukeelmers

This comment has been minimized.

Sign in to view
@stacey-gammon stacey-gammon added bug Fixes for quality problems that affect the customer experience and removed :pipeline-bugs labels Feb 13, 2019
@elastic elastic deleted a comment from elasticmachine Feb 14, 2019
@lukeelmers
Copy link
Member Author

retest

@elasticmachine
Copy link
Contributor

💔 Build Failed

@lukeelmers lukeelmers force-pushed the fix/vis-loader-errors branch from 0f551b9 to 6c703b6 Compare February 15, 2019 17:23
@elasticmachine
Copy link
Contributor

💚 Build Succeeded

ppisljar
ppisljar approved these changes Feb 21, 2019
View reviewed changes
Copy link
Member

@ppisljar ppisljar left a comment

Choose a reason for hiding this comment

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

LGTM, tested on chrome linux

emmacunningham
emmacunningham approved these changes Feb 22, 2019
View reviewed changes
Copy link

@emmacunningham emmacunningham left a comment

Choose a reason for hiding this comment

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

tested locally on Firefox (OS X); code LGTM

@lukeelmers lukeelmers merged commit f4d93ec into elastic:master Feb 22, 2019
@lukeelmers lukeelmers deleted the fix/vis-loader-errors branch February 22, 2019 01:43
@lukeelmers lukeelmers mentioned this pull request Feb 22, 2019
Merged
lukeelmers added a commit to lukeelmers/kibana that referenced this pull request Feb 22, 2019
@lukeelmers
Surface vis loader errors in the UI. (elastic#30594)
e1e7218
@lukeelmers lukeelmers mentioned this pull request Feb 22, 2019
Merged
lukeelmers added a commit to lukeelmers/kibana that referenced this pull request Feb 22, 2019
@lukeelmers
Surface vis loader errors in the UI. (elastic#30594)
66dd523
lukeelmers added a commit that referenced this pull request Feb 22, 2019
@lukeelmers
Surface vis loader errors in the UI. (#30594) (#31812)
2fe0f0d
lukeelmers added a commit that referenced this pull request Feb 22, 2019
@lukeelmers
Surface vis loader errors in the UI. (#30594) (#31811)
859a29c
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ppisljar ppisljar ppisljar approved these changes

@emmacunningham emmacunningham emmacunningham approved these changes

@timroes timroes Awaiting requested review from timroes

Assignees
No one assigned
Labels
bug Fixes for quality problems that affect the customer experience Feature:ExpressionLanguage Interpreter expression language (aka canvas pipeline) Team:Visualizations Visualization editors, elastic-charts and infrastructure v7.0.0 v7.2.0 v8.0.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Visualization query errors hidden
5 participants
@lukeelmers @elasticmachine @emmacunningham @ppisljar @stacey-gammon