fix(sqllab): error with lazy_gettext for tab titles #25469

nytai · 2023-09-29T21:22:59Z

SUMMARY

Theres an issue with creating new tabs and using the default "Untitled Query" name, db engines cannot serialize LazyString types .

Example error sqlalchemy.exc.ProgrammingError: (psycopg2.ProgrammingError) can't adapt type 'LazyString'

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

TESTING INSTRUCTIONS

open SQLLab using a query params, eg /superset/sqllab/?dbid=1&schema=default&sql=SELECT%20*%20from%20foo. Without this change it results in an error trying to create a new tab.

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

codecov · 2023-09-29T21:26:37Z

Codecov Report

Merging #25469 (61a87c0) into master (0cebffd) will decrease coverage by 10.32%.
The diff coverage is n/a.

@@             Coverage Diff             @@
##           master   #25469       +/-   ##
===========================================
- Coverage   69.10%   58.78%   -10.32%     
===========================================
  Files        1909     1909               
  Lines       74890    74890               
  Branches     8291     8291               
===========================================
- Hits        51750    44025     -7725     
- Misses      20998    28723     +7725     
  Partials     2142     2142

Flag	Coverage Δ
hive	`53.74% <ø> (ø)`
mysql	`?`
postgres	`?`
presto	`53.69% <ø> (ø)`
python	`61.55% <ø> (-21.38%)`	⬇️
sqlite	`?`
unit	`55.41% <ø> (ø)`

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

Files	Coverage Δ
superset/views/sql_lab/views.py	`52.94% <ø> (-8.09%)`	⬇️

... and 292 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

villebro · 2023-09-29T21:30:41Z

superset/views/sql_lab/views.py

@@ -90,7 +90,7 @@ def post(self) -> FlaskResponse:
            user_id=get_user_id(),
            # This is for backward compatibility
            label=query_editor.get("name")
-            or query_editor.get("title", _("Untitled Query")),
+            or query_editor.get("title", str(_("Untitled Query"))),


To simplify could we just gettext? I don't think this needs to be lazy.

villebro

LGTM with style nit

villebro · 2023-09-29T21:40:09Z

superset/views/sql_lab/views.py

@@ -21,7 +21,7 @@
 from flask_appbuilder import expose
 from flask_appbuilder.models.sqla.interface import SQLAInterface
 from flask_appbuilder.security.decorators import has_access, has_access_api
-from flask_babel import lazy_gettext as _
+from flask_babel import gettext, lazy_gettext as _


nit: I think we usually alias like so

from flask_babel import gettext as __, lazy_gettext as _

(cherry picked from commit ddde178)

* fix: is_select with UNION (apache#25290) (cherry picked from commit bb002d6) * fix: Add explicit ON DELETE CASCADE for dashboard_roles (apache#25320) (cherry picked from commit d54e827) * fix(chart): Supporting custom SQL as temporal x-axis column with filter (apache#25126) Co-authored-by: Kamil Gabryjelski <kamil.gabryjelski@gmail.com> * fix: Use RLS clause instead of ID for cache key (apache#25229) (cherry picked from commit fba66c6) * fix: Improve the reliability of alerts & reports (apache#25239) (cherry picked from commit f672d5d) * fix: DashboardRoles cascade operation (apache#25349) (cherry picked from commit a971a28) * fix: datetime with timezone excel export (apache#25318) Co-authored-by: Michael S. Molina <70410625+michael-s-molina@users.noreply.github.com> (cherry picked from commit 5ebcd2a) * fix: Workaround for Cypress ECONNRESET error (apache#25399) (cherry picked from commit d76ff39) * fix(sqllab): invalid persisted tab state (apache#25308) (apache#25398) * fix: Rename on_delete parameter to ondelete (apache#25424) (cherry picked from commit 893b45f) * fix: preventing save button from flickering in SQL Lab (apache#25106) (cherry picked from commit 296ff17) * fix: chart import (apache#25425) (cherry picked from commit a4d8f36) * fix: swagger UI CSP error (apache#25368) (cherry picked from commit 1716b9f) * fix: smarter date formatter (apache#25404) (cherry picked from commit f0080f9) * fix(sqllab): invalid start date (apache#25437) * fix(nativeFilters): Speed up native filters by removing unnecessary rerenders (apache#25282) Co-authored-by: JUST.in DO IT <justin.park@airbnb.com> (cherry picked from commit a0eeb4d) * fix(SqlLab): make icon placement even (apache#25372) (cherry picked from commit 11b49a6) * fix: Duplicate items when pasting into Select (apache#25447) (cherry picked from commit 7cf96cd) * fix: update the SQLAlchemy model definition at json column for Log table (apache#25445) (cherry picked from commit e83a76a) * fix(helm chart): set chart appVersion to 3.0.0 (apache#25373) * fix(mysql): handle string typed decimal results (apache#24241) (cherry picked from commit 7eab59a) * fix: Styles not loading because of faulty CSP setting (apache#25468) (cherry picked from commit 0cebffd) * fix(sqllab): error with lazy_gettext for tab titles (apache#25469) (cherry picked from commit ddde178) * fix: Address Mypy issue which is causing CI to fail (apache#25494) (cherry picked from commit 36ed617) * chore: Adds 3.0.1 CHANGELOG * fix: Unable to sync columns when database or dataset name contains `+` (apache#25390) (cherry picked from commit dbe0838) * fix(sqllab): Broken query containing 'children' (apache#25490) (cherry picked from commit b92957e) * chore: Expand error detail on screencapture (apache#25519) (cherry picked from commit ba541e8) * fix: tags permissions error message (apache#25516) (cherry picked from commit 50b0816) * fix: Apply normalization to all dttm columns (apache#25147) (cherry picked from commit 58fcd29) * fix: REST API CSRF exempt list (apache#25590) (cherry picked from commit 549abb5) * fix(RLS): Fix Info Tooltip + Button Alignment on RLS Modal (apache#25400) (cherry picked from commit a6d0e6f) * fix: thubmnails loading - Talisman default config (apache#25486) (cherry picked from commit 52f631a) * fix(Presto): catch DatabaseError when testing Presto views (apache#25559) Co-authored-by: Rui Zhao <zhaorui@dropbox.com> (cherry picked from commit be3714e) * fix(Charts): Set max row limit + removed the option to use an empty row limit value (apache#25579) (cherry picked from commit f556ef5) * fix(window): unavailable localStorage and sessionStorage (apache#25599) * fix: finestTemporalGrainFormatter (apache#25618) (cherry picked from commit 62bffaf) * fix: revert fix(sqllab): Force trino client async execution (apache#24859) (apache#25541) (cherry picked from commit e56e0de) * chore: Updates 3.0.1 CHANGELOG * fix(sqllab): Mistitled for new tab after rename (apache#25523) (cherry picked from commit a520124) * fix(sqllab): template validation error within comments (apache#25626) (cherry picked from commit b370c66) * fix: avoid 500 errors with SQLLAB_BACKEND_PERSISTENCE (apache#25553) (cherry picked from commit 99f79f5) * fix(import): Make sure query context is overwritten for overwriting imports (apache#25493) (cherry picked from commit a0a0d80) * fix: permalink save/overwrites in explore (apache#25112) Co-authored-by: Elizabeth Thompson <eschutho@gmail.com> (cherry picked from commit e58a3ab) * fix(header navlinks): link navlinks to path prefix (apache#25495) (cherry picked from commit 51c56dd) * fix: improve upload ZIP file validation (apache#25658) * fix: warning of nth-child (apache#23638) (cherry picked from commit 16cc089) * fix(dremio): Fixes issue with Dremio SQL generation for Charts with Series Limit (apache#25657) (cherry picked from commit be82657) --------- Co-authored-by: Beto Dealmeida <roberto@dealmeida.net> Co-authored-by: John Bodley <4567245+john-bodley@users.noreply.github.com> Co-authored-by: Zef Lin <zef@preset.io> Co-authored-by: Kamil Gabryjelski <kamil.gabryjelski@gmail.com> Co-authored-by: Jack Fragassi <jfragassi98@gmail.com> Co-authored-by: Michael S. Molina <70410625+michael-s-molina@users.noreply.github.com> Co-authored-by: JUST.in DO IT <justin.park@airbnb.com> Co-authored-by: Jack <41238731+fisjac@users.noreply.github.com> Co-authored-by: Daniel Vaz Gaspar <danielvazgaspar@gmail.com> Co-authored-by: Stepan <66589759+Always-prog@users.noreply.github.com> Co-authored-by: Corbin Bullard <corbindbullard@gmail.com> Co-authored-by: Gyuil Han <cnabro91@gmail.com> Co-authored-by: Celalettin Calis <celalettin1286@gmail.com> Co-authored-by: Ville Brofeldt <33317356+villebro@users.noreply.github.com> Co-authored-by: ʈᵃᵢ <tdupreetan@gmail.com> Co-authored-by: Michael S. Molina <michael.s.molina@gmail.com> Co-authored-by: mapledan <mapledan829@gmail.com> Co-authored-by: Igor Khrol <khroliz@gmail.com> Co-authored-by: Rui Zhao <105950525+zhaorui2022@users.noreply.github.com> Co-authored-by: Fabien <18534166+frassinier@users.noreply.github.com> Co-authored-by: Hugh A. Miles II <hughmil3s@gmail.com> Co-authored-by: OskarNS <soerensen.oskar@gmail.com>

fix(sqllab): error with lazy_gettext for tab titles

61a87c0

pull-request-size bot added the size/XS label Sep 29, 2023

villebro reviewed Sep 29, 2023

View reviewed changes

villebro approved these changes Sep 29, 2023

View reviewed changes

use gettext

38fbe46

nytai force-pushed the nytai-patch-2 branch from 8ec6785 to 38fbe46 Compare September 29, 2023 21:42

remove unused import

24bf146

nytai merged commit ddde178 into master Sep 30, 2023
54 checks passed

michael-s-molina added the v3.0 Label added by the release manager to track PRs to be included in the 3.0 branch label Oct 2, 2023

michael-s-molina pushed a commit that referenced this pull request Oct 2, 2023

fix(sqllab): error with lazy_gettext for tab titles (#25469)

455b3d8

(cherry picked from commit ddde178)

cccs-rc pushed a commit to CybercentreCanada/superset that referenced this pull request Mar 6, 2024

fix(sqllab): error with lazy_gettext for tab titles (apache#25469)

a8d42f7

mistercrunch added 🍒 3.0.1 🍒 3.0.2 🍒 3.0.3 🍒 3.0.4 🏷️ 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

mistercrunch deleted the nytai-patch-2 branch March 26, 2024 17:59

vinothkumar66 pushed a commit to vinothkumar66/superset that referenced this pull request Nov 11, 2024

fix(sqllab): error with lazy_gettext for tab titles (apache#25469)

453e06a

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(sqllab): error with lazy_gettext for tab titles #25469

fix(sqllab): error with lazy_gettext for tab titles #25469

nytai commented Sep 29, 2023

codecov bot commented Sep 29, 2023 •

edited

Loading

villebro Sep 29, 2023

villebro left a comment

villebro Sep 29, 2023

fix(sqllab): error with lazy_gettext for tab titles #25469

fix(sqllab): error with lazy_gettext for tab titles #25469

Conversation

nytai commented Sep 29, 2023

SUMMARY

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

TESTING INSTRUCTIONS

ADDITIONAL INFORMATION

codecov bot commented Sep 29, 2023 • edited Loading

Codecov Report

villebro Sep 29, 2023

Choose a reason for hiding this comment

villebro left a comment

Choose a reason for hiding this comment

villebro Sep 29, 2023

Choose a reason for hiding this comment

codecov bot commented Sep 29, 2023 •

edited

Loading