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

feat: add Query.columns for bootstrap_data #20134

Merged
merged 1 commit into from
May 26, 2022

Conversation

hughhhh
Copy link
Member

@hughhhh hughhhh commented May 19, 2022

SUMMARY

Added logic to send Query.columns to explore if the datasource is a Query.

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

TESTING INSTRUCTIONS

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
Copy link

codecov bot commented May 19, 2022

Codecov Report

Merging #20134 (8cefeb6) into master (660af40) will decrease coverage by 11.91%.
The diff coverage is 28.57%.

❗ Current head 8cefeb6 differs from pull request most recent head 433d4d7. Consider uploading reports for the commit 433d4d7 to get more accurate results

@@             Coverage Diff             @@
##           master   #20134       +/-   ##
===========================================
- Coverage   66.47%   54.55%   -11.92%     
===========================================
  Files        1721     1721               
  Lines       64477    64500       +23     
  Branches     6795     6805       +10     
===========================================
- Hits        42858    35187     -7671     
- Misses      19891    27581     +7690     
- Partials     1728     1732        +4     
Flag Coverage Δ
hive 53.70% <0.00%> (-0.01%) ⬇️
mysql ?
postgres ?
presto 53.56% <0.00%> (-0.01%) ⬇️
python 57.99% <40.00%> (-24.64%) ⬇️
sqlite ?
unit 49.40% <40.00%> (-0.01%) ⬇️

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

Impacted Files Coverage Δ
...egacy-plugin-chart-event-flow/src/controlPanel.tsx 14.28% <0.00%> (ø)
...chart-echarts/src/MixedTimeseries/controlPanel.tsx 86.66% <0.00%> (-6.20%) ⬇️
.../plugin-chart-echarts/src/MixedTimeseries/index.ts 25.00% <0.00%> (-25.00%) ⬇️
...hart-echarts/src/MixedTimeseries/transformProps.ts 0.00% <0.00%> (ø)
.../plugin-chart-echarts/src/Timeseries/buildQuery.ts 66.66% <ø> (-4.77%) ⬇️
...tend/plugins/plugin-chart-echarts/src/controls.tsx 56.66% <0.00%> (-17.25%) ⬇️
...nd/src/explore/controlUtils/getSectionsToRender.ts 80.00% <ø> (-0.96%) ⬇️
superset/views/core.py 35.06% <0.00%> (-42.33%) ⬇️
...d/plugins/plugin-chart-echarts/src/utils/series.ts 87.50% <20.00%> (-4.50%) ⬇️
...t-frontend/src/dashboard/components/SliceAdder.jsx 66.17% <33.33%> (-3.18%) ⬇️
... and 286 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 660af40...433d4d7. Read the comment docs.

@hughhhh hughhhh changed the title add Query.columns for bootstrap_data feat: add Query.columns for bootstrap_data May 25, 2022
@hughhhh hughhhh force-pushed the add-columns-bootstrap-data branch from d1358dd to 433d4d7 Compare May 25, 2022 23:03
Copy link
Member

@pkdotson pkdotson left a comment

Choose a reason for hiding this comment

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

lgtm!

@hughhhh hughhhh merged commit 7a2eb8b into apache:master May 26, 2022
@@ -905,6 +908,7 @@ def explore(
"user": bootstrap_user_data(g.user, include_perms=True),
"forced_height": request.args.get("height"),
"common": common_bootstrap_payload(),
"columns": columns,
Copy link
Member

Choose a reason for hiding this comment

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

The current columns are passed in to the datasource object. For easier lookup, do you think we can keep the same structure for queries, instead of having to look in two different places?

philipher29 pushed a commit to ValtechMobility/superset that referenced this pull request Jun 9, 2022
@mistercrunch mistercrunch added 🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels 🚢 2.0.0 labels Mar 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels size/XS 🚢 2.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants