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

fix(sqllab): misplaced limit warning alert #25306

Merged
merged 2 commits into from
Sep 27, 2023
Merged

fix(sqllab): misplaced limit warning alert #25306

merged 2 commits into from
Sep 27, 2023

Conversation

justinpark
Copy link
Member

@justinpark justinpark commented Sep 14, 2023 •
edited
Loading

SUMMARY

Hotfix for #25288

This commit fixes the displaced limit warning alert in the query result.

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

After Before
Screenshot 2023-09-14 at 11 14 24 AM Notification_Center

TESTING INSTRUCTIONS

  1. Go to sql, run query without selecting the limit or set limit in query
  2. Run query
  3. Observe the the result section
  4. See the yellow banner is to the right side of the query

ADDITIONAL INFORMATION

  • Has associated issue:
  • Required feature flags:
  • Changes UI
  • Includes DB Migration (follow approval process in SIP-59)
    • Migration is atomic, supports rollback & is backwards-compatible
    • Confirm DB migration upgrade and downgrade tested
    • Runtime estimates and downtime expectations provided
  • Introduces new feature or API
  • Removes existing feature or API

@jinghua-qa
Copy link
Member

/testenv up

@github-actions
Copy link
Contributor

@jinghua-qa Ephemeral environment spinning up at http://52.12.66.159:8080. Credentials are admin/admin. Please allow several minutes for bootstrapping and startup.

@jinghua-qa
Copy link
Member

jinghua-qa commented Sep 18, 2023 •
edited
Loading

I tested in the ephemeral, the original banner issue is fixed, however i noticed that when i try to change dropdown limit to returning larger rows, seems like there is a error yellow banner about no stored result found, and then it will take couple seconds for the result to load.

Screen Shot 2023-09-18 at 9 41 47 AM
Screen.Recording.2023-09-18.at.9.54.45.AM.mov

@justinpark
Copy link
Member Author

@jinghua-qa I push a commit to fix the issue. could you rebuild ephemeral env and test again?

I tested in the ephemeral, the original banner issue is fixed, however i noticed that when i try to change dropdown limit to returning larger rows, seems like there is a error yellow banner about no stored result found, and then it will take couple seconds for the result to load.

Screen Shot 2023-09-18 at 9 41 47 AM Screen.Recording.2023-09-18.at.9.54.45.AM.mov

@michael-s-molina
Copy link
Member

/testenv up

@github-actions
Copy link
Contributor

@michael-s-molina Ephemeral environment spinning up at http://52.26.29.16:8080. Credentials are admin/admin. Please allow several minutes for bootstrapping and startup.

@jinghua-qa
Copy link
Member

Tested the ephemeral env looks like the issue i found is fixed. LGTM!

Antonio-RiveroMartnez
Antonio-RiveroMartnez approved these changes Sep 26, 2023
View reviewed changes
Copy link
Member

@Antonio-RiveroMartnez Antonio-RiveroMartnez left a comment

Choose a reason for hiding this comment

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

lgtm

@justinpark justinpark merged commit 463962a into apache:master Sep 27, 2023
26 checks passed
@github-actions
Copy link
Contributor

Ephemeral environment shutdown and build artifacts deleted.

sadpandajoe pushed a commit to preset-io/superset that referenced this pull request Sep 27, 2023
@justinpark @sadpandajoe
fix(sqllab): misplaced limit warning alert (apache#25306)
9cf28f0 
(cherry picked from commit 463962a)
@sadpandajoe
Copy link
Member

🏷️ preset:2023.39

@justinpark justinpark mentioned this pull request Oct 31, 2023
Merged
9 tasks
cccs-rc pushed a commit to CybercentreCanada/superset that referenced this pull request Mar 6, 2024
@justinpark
fix(sqllab): misplaced limit warning alert (apache#25306)
479ee9f
@mistercrunch mistercrunch added 🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels 🚢 3.1.0 labels Mar 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Antonio-RiveroMartnez Antonio-RiveroMartnez Antonio-RiveroMartnez approved these changes

@michael-s-molina michael-s-molina Awaiting requested review from michael-s-molina

@jinghua-qa jinghua-qa Awaiting requested review from jinghua-qa

Assignees
No one assigned
Labels
🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels size/L 🚢 3.1.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@justinpark @jinghua-qa @michael-s-molina @sadpandajoe @Antonio-RiveroMartnez @mistercrunch