-
Notifications
You must be signed in to change notification settings - Fork 14.1k
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(sqllab): flaky json explore modal due to over-rendering #26791
fix(sqllab): flaky json explore modal due to over-rendering #26791
Conversation
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## master #26791 +/- ##
=======================================
Coverage 67.19% 67.19%
=======================================
Files 1899 1900 +1
Lines 74368 74370 +2
Branches 8274 8277 +3
=======================================
+ Hits 49969 49973 +4
+ Misses 22344 22343 -1
+ Partials 2055 2054 -1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
/testenv up |
@justinpark Container image not yet published for this PR. Please try again when build is complete. |
@justinpark Ephemeral environment creation failed. Please check the Actions logs for details. |
/testenv up |
@michael-s-molina Container image not yet published for this PR. Please try again when build is complete. |
@michael-s-molina Ephemeral environment creation failed. Please check the Actions logs for details. |
cc @gaurav7261 |
39a1846
to
dbf0232
Compare
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.
This looks solid overall, well covered by tests too.
dbf0232
to
7e6c7bc
Compare
7e6c7bc
to
821a709
Compare
/testenv up |
@justinpark Ephemeral environment spinning up at http://54.245.8.160:8080. Credentials are |
@justinpark any reason not to merge this? |
Ephemeral environment shutdown and build artifacts deleted. |
SUMMARY
This commit reopens #26156, which resolves the issue detailed in #26276 by excluding the changes in 'QueryAutoRefresh/index.tsx' and 'reducers/sqlLab.js'.
This commit also reverses the changes introduced in #18934. Now that the issue of overrendering due to redux has been resolved, it also effectively addresses the problem outlined in issue #26755.
BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
Before:
before--overrendering.mov
After:
after--overrendering.mov
TESTING INSTRUCTIONS
Go to SQL Lab and run a query contains a json blob
Select a table in order to execute a table preview query
Click
Results
tab before the table preview query ends and then click the JSON column to open the JSON modalCheck the JSON modal persists while the async query status is updating
ADDITIONAL INFORMATION