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

revert: "fix(sql lab): display the 'View Results' button consistently in the history tab on sync mode" #19906

Merged

Conversation

Gwitchr
Copy link
Contributor

@Gwitchr Gwitchr commented Apr 29, 2022

Reverts #19362

…in the history tab on sync mode (apache#19362)"

This reverts commit 6d5d03e.
@codecov
Copy link

codecov bot commented Apr 29, 2022

Codecov Report

Merging #19906 (bdf10ad) into master (aff10a7) will increase coverage by 0.00%.
The diff coverage is 100.00%.

❗ Current head bdf10ad differs from pull request most recent head 1a9c031. Consider uploading reports for the commit 1a9c031 to get more accurate results

@@           Coverage Diff           @@
##           master   #19906   +/-   ##
=======================================
  Coverage   66.52%   66.53%           
=======================================
  Files        1714     1714           
  Lines       65052    65051    -1     
  Branches     6722     6722           
=======================================
+ Hits        43279    43280    +1     
+ Misses      20061    20060    -1     
+ Partials     1712     1711    -1     
Flag Coverage Δ
javascript 51.25% <100.00%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
superset-frontend/src/SqlLab/reducers/sqlLab.js 33.15% <ø> (ø)
superset/sql_lab.py 81.64% <ø> (+0.31%) ⬆️
...frontend/src/SqlLab/components/ResultSet/index.tsx 50.49% <100.00%> (+0.49%) ⬆️

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 aff10a7...1a9c031. Read the comment docs.

@Gwitchr Gwitchr changed the title revert: "fix(sql lab): display the 'View Results' button consistently in the history tab on sync mode" fix: "fix(sql lab): display the 'View Results' button consistently in the history tab on sync mode" May 2, 2022
@rusackas
Copy link
Member

rusackas commented May 2, 2022

For added context, this PR is reverting the prior fix because it led to a new issue, where results from valid queries were not being displayed properly. To repro:

  1. Run sql query with 100 rows limit
  2. Check query history and observe that there is no "view result" button
  3. Run the query again with different row limit, and see that the result button is now shown

Copy link
Member

@rusackas rusackas left a comment

Choose a reason for hiding this comment

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

OK to revert... let's try to fix forward and solve the original problem though.

@rusackas rusackas merged commit 1fa841e into apache:master May 2, 2022
@rusackas rusackas changed the title fix: "fix(sql lab): display the 'View Results' button consistently in the history tab on sync mode" revert: "fix(sql lab): display the 'View Results' button consistently in the history tab on sync mode" May 2, 2022
@sadpandajoe
Copy link
Member

🏷️ preset:2022.17

sadpandajoe pushed a commit to preset-io/superset that referenced this pull request May 2, 2022
…in the history tab on sync mode (apache#19362)" (apache#19906)

This reverts commit 6d5d03e.

(cherry picked from commit 1fa841e)
hughhhh pushed a commit to hve-labs/superset that referenced this pull request May 11, 2022
philipher29 pushed a commit to ValtechMobility/superset that referenced this pull request Jun 9, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants