refactor: migrate table chart to new API by ktmud · Pull Request #10270 · apache/superset

ktmud · 2020-07-09T16:27:26Z

SUMMARY

Move the table viz to API v1. Depends on apache-superset/superset-ui#889

Also did a bunch of refactoring on QueryContext and QueryObject:

Rename DbColumnTypes to GenericDataType
Add colnames and coltypes to query response. Didn't call them column_names to make it easier to search in the code.
Support DTTM_ALIAS in groupby/columns
Add a query context validation function, which runs more advanced validation on query context parameters before running the query. Currently only checks for duplicate column labels.
Update post_processing.contribution to support applying contribution only on selected columns (needed for percent metrics).

Bumped superset-ui packages and plugins to 0.17.0:
apache-superset/superset-ui@v0.16.9...v0.17.0

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

N/A

TEST PLAN

CI and manual verification.

Make sure major charts don't break.
Remember to test other charts that uses API v1, including word cloud, echarts, etc.

ADDITIONAL INFORMATION

Has associated issue: feat: bump superset-ui and implement queryFields in formData #9908 feat: implement new version of word cloud #9962
Changes UI
Requires DB Migration.
Confirm DB Migration upgrade and downgrade tested.
Introduces new feature or API
Removes existing feature or API

villebro · 2020-07-23T18:24:08Z

@ktmud this needs a rebase, will be happy to give it a spin once the conflicts are resolved

villebro

Minor comment on adding the new results to the response schema. Looks great, let's get this in as soon as superset-ui/core is bumped to 0.15.

stale · 2020-11-26T22:17:14Z

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. For admin, please label this issue .pinned to prevent stale bot from closing the issue.

codecov-io · 2020-12-10T22:24:41Z

Codecov Report

Merging #10270 (5cfab09) into master (bab86ab) will increase coverage by 1.95%.
The diff coverage is 84.28%.

@@            Coverage Diff             @@
##           master   #10270      +/-   ##
==========================================
+ Coverage   64.99%   66.94%   +1.95%     
==========================================
  Files        1021     1021              
  Lines       50095    50125      +30     
  Branches     5141     5141              
==========================================
+ Hits        32559    33556     +997     
+ Misses      17360    16435     -925     
+ Partials      176      134      -42

Flag	Coverage Δ
cypress	`50.87% <ø> (+13.23%)`	⬆️
javascript	`62.00% <ø> (ø)`
python	`63.87% <84.28%> (-0.11%)`	⬇️

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

Impacted Files	Coverage Δ
superset/charts/api.py	`82.49% <66.66%> (-0.20%)`	⬇️
superset/common/query_object.py	`89.78% <71.42%> (-2.09%)`	⬇️
superset/models/core.py	`87.77% <75.00%> (-0.82%)`	⬇️
superset/common/query_context.py	`82.14% <77.77%> (-0.45%)`	⬇️
superset/utils/core.py	`88.04% <90.00%> (-0.01%)`	⬇️
superset/connectors/sqla/models.py	`90.58% <100.00%> (ø)`
superset/db_engine_specs/base.py	`85.45% <100.00%> (-0.52%)`	⬇️
superset/utils/pandas_postprocessing.py	`79.11% <100.00%> (+0.51%)`	⬆️
superset/sql_validators/postgres.py	`50.00% <0.00%> (-50.00%)`	⬇️
superset/databases/commands/create.py	`83.67% <0.00%> (-8.17%)`	⬇️
... and 109 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 bab86ab...5cfab09. Read the comment docs.

villebro · 2020-12-21T09:38:52Z

@ktmud if you're busy I can help finish this

ktmud · 2020-12-21T19:34:31Z

@villebro I'll finish it this week.

ktmud · 2021-01-14T23:43:56Z

Use normalized_messsages from mashmallow's ValidationError so that the error message shows which query field failed the validation.

ktmud · 2021-01-26T21:06:09Z

HOLD: Sort by isn't working for aggregation mode.

We may need another update on superset-ui.

(Thanks for the Cypress tests!)

ktmud · 2021-01-27T08:40:04Z

@villebro @zhaoyongjie An update on this. I didn't have time to fix this today but will try to do it tomorrow. This PR should block bumping superset-ui packages since apache-superset/superset-ui#889 has been published.

ktmud · 2021-01-29T02:23:23Z

@kristw @villebro could you help taking a look at apache-superset/superset-ui#919 , which should fix the failing E2E test in this PR.

villebro · 2021-01-29T09:06:28Z

could you help taking a look at apache-superset/superset-ui#919 , which should fix the failing E2E test in this PR.

Approved - I'll merge it and deploy in an hour or so if you're offline (hmm, it seems to be stuck, I'll kick CI if it doesn't wake up soon)

Fix time column formatting test

ktmud · 2021-01-29T11:08:58Z

@villebro I've done the bumping. Feel free to merge when the CI is green.

villebro · 2021-01-29T11:12:03Z

That would be right now 🙂

* refactor: migrate table chart to new API * chore: bump superset-ui to 0.17.0 * Fix Cypress tests * Apply soft-conversion to numeric metrics Fix time column formatting test * Add translation to chart does not exist error * Bump to 0.17.1

pull-request-size Bot added the size/L label Jul 9, 2020

ktmud force-pushed the table-chart-new-api branch from afc2b4a to 0273508 Compare July 9, 2020 16:29

rusackas requested a review from villebro July 23, 2020 17:45

villebro requested changes Sep 9, 2020

View reviewed changes

Comment thread superset/connectors/sqla/models.py Outdated

Comment thread superset/db_engine_specs/base.py Outdated

Comment thread superset/utils/core.py Outdated

Comment thread superset/common/query_context.py Outdated

ktmud force-pushed the table-chart-new-api branch from 0273508 to 602dce7 Compare September 14, 2020 04:31

stale Bot added the inactive Inactive for >= 30 days label Nov 26, 2020

ktmud force-pushed the table-chart-new-api branch from 602dce7 to a1e87a8 Compare November 27, 2020 05:01

stale Bot removed the inactive Inactive for >= 30 days label Nov 27, 2020

ktmud force-pushed the table-chart-new-api branch from a1e87a8 to 295c88d Compare December 10, 2020 21:57

ktmud force-pushed the table-chart-new-api branch 3 times, most recently from 7be3fd5 to 4fcf60e Compare January 7, 2021 21:33

ktmud mentioned this pull request Jan 7, 2021

refactor(plugin-chart-table): migrate to API v1 apache-superset/superset-ui#889

Merged

ktmud changed the title ~~[WIP] refactor: migrate table chart to new API~~ refactor: migrate table chart to new API Jan 7, 2021

ktmud force-pushed the table-chart-new-api branch from 1129349 to a0fdb6e Compare January 8, 2021 00:10

ktmud mentioned this pull request Jan 10, 2021

[Explore] Saved metrics label overrides not working #12375

Closed

3 tasks

ktmud force-pushed the table-chart-new-api branch 3 times, most recently from f46ee03 to 5ee9045 Compare January 14, 2021 21:51

ktmud marked this pull request as ready for review January 14, 2021 22:01

ktmud commented Jan 14, 2021

View reviewed changes

ktmud force-pushed the table-chart-new-api branch from 8e2dc63 to 210ae65 Compare January 15, 2021 01:03

ktmud mentioned this pull request Jan 15, 2021

fix(viz): missing groupby and broken adhoc metrics for boxplot #12556

Merged

6 tasks

ktmud force-pushed the table-chart-new-api branch 2 times, most recently from 84d92b3 to 8263bf7 Compare January 15, 2021 21:10

ktmud force-pushed the table-chart-new-api branch 2 times, most recently from 46cb0cb to 9b05065 Compare January 26, 2021 10:39

villebro approved these changes Jan 26, 2021

View reviewed changes

ktmud force-pushed the table-chart-new-api branch from 588ceee to 026c639 Compare January 26, 2021 17:57

pull-request-size Bot added size/XL and removed size/L labels Jan 26, 2021

ktmud added the hold! On hold label Jan 27, 2021

ktmud mentioned this pull request Jan 28, 2021

feat(core): add orderby to extractQueryFields apache-superset/superset-ui#919

Merged

ktmud added 6 commits January 29, 2021 00:16

refactor: migrate table chart to new API

ae2e7ca

chore: bump superset-ui to 0.17.0

6866905

Fix Cypress tests

133ef22

Apply soft-conversion to numeric metrics

87f8d01

Fix time column formatting test

Add translation to chart does not exist error

f1aa1f6

Bump to 0.17.1

5cfab09

ktmud force-pushed the table-chart-new-api branch from ef12444 to 5cfab09 Compare January 29, 2021 10:17

villebro merged commit e3db935 into apache:master Jan 29, 2021

rusackas removed the hold! On hold label Feb 2, 2021

villebro mentioned this pull request Feb 2, 2021

fix(explore): missing select when groupby without metrics #12890

Merged

6 tasks

ktmud mentioned this pull request Feb 4, 2021

test: fix table chart sort order E2E tests #12936

Merged

6 tasks

This was referenced Feb 4, 2021

fix(viz): improve dtype inference logic #12933

Merged

[table] Total Row for Table Charts v2(in comment) #12907

Closed

mistercrunch added 🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels 🚢 1.2.0 First shipped in 1.2.0 labels Feb 19, 2024

Uh oh!

Conversation

ktmud commented Jul 9, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

SUMMARY

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

TEST PLAN

ADDITIONAL INFORMATION

Uh oh!

villebro commented Jul 23, 2020

Uh oh!

villebro left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

stale Bot commented Nov 26, 2020

Uh oh!

codecov-io commented Dec 10, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

villebro commented Dec 21, 2020

Uh oh!

ktmud commented Dec 21, 2020

Uh oh!

ktmud Jan 14, 2021

Choose a reason for hiding this comment

Uh oh!

ktmud commented Jan 26, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ktmud commented Jan 27, 2021

Uh oh!

ktmud commented Jan 29, 2021

Uh oh!

villebro commented Jan 29, 2021

Uh oh!

ktmud commented Jan 29, 2021

Uh oh!

villebro commented Jan 29, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

ktmud commented Jul 9, 2020 •

edited

Loading

codecov-io commented Dec 10, 2020 •

edited

Loading

ktmud commented Jan 26, 2021 •

edited

Loading