fix: Support Jinja template functions in global async queries #16412

robdiciuccio · 2021-08-23T22:09:27Z

SUMMARY

Makes Jinja template processing compatible with Global Async Queries and Celery-based processing.

TESTING INSTRUCTIONS

Test Jinja template functions url_param and filter_values in Dashboards and Explore via custom SQL templates, with GLOBAL_ASYNC_QUERIES feature flag enabled and disabled.

ADDITIONAL INFORMATION

Has associated issue: Jinja filters not working with Global Async Queries (GAQ) #14786
Required feature flags: GLOBAL_ASYNC_QUERIES
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 · 2021-08-23T22:25:12Z

Codecov Report

Merging #16412 (284aa48) into master (bc4b6f0) will decrease coverage by 0.13%.
The diff coverage is 79.72%.

❗ Current head 284aa48 differs from pull request most recent head 35e50fe. Consider uploading reports for the commit 35e50fe to get more accurate results

@@            Coverage Diff             @@
##           master   #16412      +/-   ##
==========================================
- Coverage   76.57%   76.43%   -0.14%     
==========================================
  Files        1000     1002       +2     
  Lines       53489    53698     +209     
  Branches     6816     6816              
==========================================
+ Hits        40957    41046      +89     
- Misses      12296    12416     +120     
  Partials      236      236

Flag	Coverage Δ
hive	`?`
mysql	`81.50% <84.21%> (-0.02%)`	⬇️
postgres	`81.57% <84.35%> (+0.02%)`	⬆️
python	`81.62% <84.35%> (-0.30%)`	⬇️
sqlite	`?`

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

Impacted Files	Coverage Δ
...erset-frontend/src/SqlLab/components/ResultSet.tsx	`62.45% <ø> (ø)`
...-frontend/src/components/OmniContainer/Omnibar.tsx	`100.00% <ø> (ø)`
superset-frontend/src/components/Select/Select.tsx	`73.42% <ø> (ø)`
...et-frontend/src/components/TableView/TableView.tsx	`94.52% <ø> (ø)`
superset-frontend/src/dashboard/actions/hydrate.js	`1.69% <0.00%> (ø)`
...et-frontend/src/dashboard/components/Dashboard.jsx	`78.84% <ø> (ø)`
...frontend/src/dashboard/components/Header/index.jsx	`69.49% <ø> (ø)`
...ashboard/components/gridComponents/ChartHolder.jsx	`68.68% <ø> (ø)`
...c/dashboard/components/gridComponents/Markdown.jsx	`82.82% <ø> (ø)`
...nd/src/dashboard/containers/DashboardComponent.jsx	`100.00% <ø> (ø)`
... and 144 more

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 bc4b6f0...35e50fe. Read the comment docs.

craig-rueda

Is there a test for this already?

robdiciuccio · 2021-08-26T18:19:24Z

@craig-rueda there is test coverage for using g.form_data here. Flask has_request_context doesn't work properly in tests, apparently due to flask._preserve_context being enabled in test_client(), and mocking it doesn't work either. ¯_(ツ)_/¯

villebro

LGTM - small nit wrt duplicated logic

villebro · 2021-08-31T10:32:18Z

superset/views/utils.py

+        request_json_data = (
+            request.json["queries"][0]
+            if (
+                has_request_context()  # type: ignore


Didn't we already check for has_request_context() on line 134?

Yep, good catch. Leftover from a refactor. I'll remove.

…#16412) * Support Jinja template functions in async queries * Pylint * Add tests for async tasks * Remove redundant has_request_context check

* Support Jinja template functions in async queries * Pylint * Add tests for async tasks * Remove redundant has_request_context check (cherry picked from commit 4e380db)

…#16412) * Support Jinja template functions in async queries * Pylint * Add tests for async tasks * Remove redundant has_request_context check

* add support for adhoc columns to api and sqla model * fix some types * fix duplicates in column names * fix more lint * fix schema and dedup * clean up some logic * first pass at fixing viz.py * Add frontend support for adhoc columns * Add title edit * Fix showing custom title * Use column name as default value in sql editor * fix: Adds a loading message when needed in the Select component (#16531) * fix(tests): make parquet select deterministic with order by (#16570) * bump emotion to help with cache clobbering (#16559) * fix: Support Jinja template functions in global async queries (#16412) * Support Jinja template functions in async queries * Pylint * Add tests for async tasks * Remove redundant has_request_context check * fix: impersonate user label/tooltip (#16573) * docs: update for small typos (#16568) * feat: Add Aurora Data API engine spec (#16535) * feat: Add Aurora Data API engine spec * Fix lint * refactor: sql_json view endpoint: encapsulate ctas parameters (#16548) * refactor sql_json view endpoint: encapsulate ctas parameters * fix failed tests * fix failed tests and ci issues * refactor sql_json view endpoint: separate concern into ad hod method (#16595) * feat: Experimental cross-filter plugins (#16594) * fix:fix get permission function * feat: add cross filter chart in charts gallery under FF * chore(deps): bump superset-ui to 0.18.2 (#16601) * update type guard references * fix imports * update series_columns schema * Add changes that got lost in rebase * Use current columns name or expression as sql editor init value * add integration test and do minor fixes * Bump superset-ui * fix linting issue * bump superset-ui to 0.18.22 * resolve merge conflict * lint * fix select filter infinite loop * bump superset-ui to 0.18.23 * Fix auto setting column popover title * Enable adhoc columns only if UX_BETA enabled * put back removed test * Move popover height and width to constants * Refactor big ternary expression Co-authored-by: Kamil Gabryjelski <kamil.gabryjelski@gmail.com> Co-authored-by: Michael S. Molina <70410625+michael-s-molina@users.noreply.github.com> Co-authored-by: Elizabeth Thompson <eschutho@gmail.com> Co-authored-by: Rob DiCiuccio <rob.diciuccio@gmail.com> Co-authored-by: Beto Dealmeida <roberto@dealmeida.net> Co-authored-by: joeADSP <75027008+joeADSP@users.noreply.github.com> Co-authored-by: ofekisr <35701650+ofekisr@users.noreply.github.com> Co-authored-by: simcha90 <56388545+simcha90@users.noreply.github.com>

…#16412) * Support Jinja template functions in async queries * Pylint * Add tests for async tasks * Remove redundant has_request_context check

* add support for adhoc columns to api and sqla model * fix some types * fix duplicates in column names * fix more lint * fix schema and dedup * clean up some logic * first pass at fixing viz.py * Add frontend support for adhoc columns * Add title edit * Fix showing custom title * Use column name as default value in sql editor * fix: Adds a loading message when needed in the Select component (#16531) * fix(tests): make parquet select deterministic with order by (#16570) * bump emotion to help with cache clobbering (#16559) * fix: Support Jinja template functions in global async queries (#16412) * Support Jinja template functions in async queries * Pylint * Add tests for async tasks * Remove redundant has_request_context check * fix: impersonate user label/tooltip (#16573) * docs: update for small typos (#16568) * feat: Add Aurora Data API engine spec (#16535) * feat: Add Aurora Data API engine spec * Fix lint * refactor: sql_json view endpoint: encapsulate ctas parameters (#16548) * refactor sql_json view endpoint: encapsulate ctas parameters * fix failed tests * fix failed tests and ci issues * refactor sql_json view endpoint: separate concern into ad hod method (#16595) * feat: Experimental cross-filter plugins (#16594) * fix:fix get permission function * feat: add cross filter chart in charts gallery under FF * chore(deps): bump superset-ui to 0.18.2 (#16601) * update type guard references * fix imports * update series_columns schema * Add changes that got lost in rebase * Use current columns name or expression as sql editor init value * add integration test and do minor fixes * Bump superset-ui * fix linting issue * bump superset-ui to 0.18.22 * resolve merge conflict * lint * fix select filter infinite loop * bump superset-ui to 0.18.23 * Fix auto setting column popover title * Enable adhoc columns only if UX_BETA enabled * put back removed test * Move popover height and width to constants * Refactor big ternary expression Co-authored-by: Kamil Gabryjelski <kamil.gabryjelski@gmail.com> Co-authored-by: Michael S. Molina <70410625+michael-s-molina@users.noreply.github.com> Co-authored-by: Elizabeth Thompson <eschutho@gmail.com> Co-authored-by: Rob DiCiuccio <rob.diciuccio@gmail.com> Co-authored-by: Beto Dealmeida <roberto@dealmeida.net> Co-authored-by: joeADSP <75027008+joeADSP@users.noreply.github.com> Co-authored-by: ofekisr <35701650+ofekisr@users.noreply.github.com> Co-authored-by: simcha90 <56388545+simcha90@users.noreply.github.com>

Support Jinja template functions in async queries

a69acde

robdiciuccio requested a review from villebro August 23, 2021 22:09

superset-github-bot bot added the preset-io label Aug 23, 2021

pull-request-size bot added the size/M label Aug 23, 2021

robdiciuccio mentioned this pull request Aug 23, 2021

Jinja filters not working with Global Async Queries (GAQ) #14786

Closed

3 tasks

Pylint

42872a6

robdiciuccio requested a review from craig-rueda August 25, 2021 23:12

craig-rueda reviewed Aug 25, 2021

View reviewed changes

Add tests for async tasks

cb2fae8

pull-request-size bot added size/L and removed size/M labels Aug 26, 2021

villebro reviewed Aug 31, 2021

View reviewed changes

Remove redundant has_request_context check

35e50fe

villebro approved these changes Sep 1, 2021

View reviewed changes

villebro merged commit 4e380db into apache:master Sep 3, 2021

villebro deleted the rd/jinja-async-queries branch September 3, 2021 11:33

villebro added the v1.3 label Sep 3, 2021

anthony6401 mentioned this pull request Sep 9, 2021

Jinja template is not working when async query is enabled #16650

Closed

3 tasks

mistercrunch added 🍒 1.3.1 🍒 1.3.2 🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels 🚢 1.4.0 labels Mar 13, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: Support Jinja template functions in global async queries #16412

fix: Support Jinja template functions in global async queries #16412

robdiciuccio commented Aug 23, 2021

codecov bot commented Aug 23, 2021 •

edited

Loading

craig-rueda left a comment

robdiciuccio commented Aug 26, 2021

villebro left a comment

villebro Aug 31, 2021

robdiciuccio Aug 31, 2021

fix: Support Jinja template functions in global async queries #16412

fix: Support Jinja template functions in global async queries #16412

Conversation

robdiciuccio commented Aug 23, 2021

SUMMARY

TESTING INSTRUCTIONS

ADDITIONAL INFORMATION

codecov bot commented Aug 23, 2021 • edited Loading

Codecov Report

craig-rueda left a comment

Choose a reason for hiding this comment

robdiciuccio commented Aug 26, 2021

villebro left a comment

Choose a reason for hiding this comment

villebro Aug 31, 2021

Choose a reason for hiding this comment

robdiciuccio Aug 31, 2021

Choose a reason for hiding this comment

codecov bot commented Aug 23, 2021 •

edited

Loading