-
Notifications
You must be signed in to change notification settings - Fork 31
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
fix: Log figure errors, don't show infinite spinner #1614
Conversation
mofojed
commented
Nov 2, 2023
•
edited
Loading
edited
- Requires Core PR Add support for MultiXYErrorBarSeries, MultiOHLCSeries figures deephaven-core#4763
- Also need to port that to Enterprise
- Actually look at the errors reported by the figure, and log them so we have a clearer idea of what the issue is when plots are reported as not working
- Also show the error message in the chart panel button bar. Clicking the button will display the full error message.
- Don't show the spinner infinitely if there are no series to load
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #1614 +/- ##
==========================================
- Coverage 46.63% 46.60% -0.03%
==========================================
Files 591 592 +1
Lines 36406 36435 +29
Branches 9113 9120 +7
==========================================
+ Hits 16979 16982 +3
- Misses 19375 19401 +26
Partials 52 52
Flags with carried forward coverage won't be shown. Click here to find out more.
☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Need instructions on how to test this if I'm using the linked core PR
@mattrunyon updated so you can see the full error message from the UI: |
Use the linked Core PR with Groovy, and test with this snippet:
I then commented out the changes in FigureWidgetTranslator part from that linked PR so that same snippet produces a plot with an error, and the error appears correctly. |
4924616
to
78451a5
Compare
- Log errors reported by the figure - If the figure doesn't have any series, don't show the loading spinner infinitely. Just show the blank chart.
- Emit error from model so the UI can handle it - It just stops the loading spinner right now - we should look at displaying a toast later - Clean up `updateGrid` calls
- Now the error appears as a button in the button bar - Clicking the button will display the full error message - Have it wired up to show the downsampling error message as well when that occurs
Co-authored-by: Matthew Runyon <mattrunyonstuff@gmail.com>
- chartTheme was null
78451a5
to
438ed4f
Compare