feat: Migrates Pivot Table v1 to v2 #23712

michael-s-molina · 2023-04-17T16:38:45Z

SUMMARY

This PR migrates the Pivot Table v1 chart to v2. In detail, it does the following:

Creates a processor for the Pivot Table chart to convert v1 form_data to v2
Augments the base MigrateViz class to support migrating generic chart axes related fields given that some legacy plugins only work without generic chart axes
Creates a database migration that invokes the processor
Creates a migration test
Removes the legacy Pivot Table code and unregisters the plugin.
Removes old plugin references throughout the codebase
Updates unit tests
Updates E2E tests
Updates the example dashboards
Updates translation files

The following table summarizes the differences in the form_data structure between the versions:

v1	v2	Notes
columns	groupbyColumns
combine_metric	combineMetric
granularity_sqla	adhoc_filters.subject	Only if GENERIC_CHART_AXES is enabled
groupby	groupbyRows
number_format	valueFormat
pandas_aggfunc	aggregateFunction	Values changed between versions
pivot_margins	rowTotals, colTotals
row_limit	series_limit
time_range	adhoc_filters.comparator	Only if GENERIC_CHART_AXES is enabled
timeseries_limit_metric	series_limit_metric
transpose_pivot	transposePivot

* All other keys are the same or exist only in the target version
* The time column type (simple or SQL) affects the adhoc_filters value

AFTER SCREENSHOT

TESTING INSTRUCTIONS

1 - Make sure all Pivot Table (legacy) charts were converted to Pivot Table
2 - Make sure Pivot Table (legacy) is not available anymore in the viz picker
3 - Make sure that it's possible to revert the migration by executing superset db downgrade and reverting this PR

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

michael-s-molina · 2023-04-19T13:48:53Z

@villebro @kgabryje Can you execute the migration in your staging environments to make sure it runs successfully and the charts are correctly migrated? I executed it successfully at Airbnb over thousands of charts.

codecov · 2023-04-19T13:55:09Z

Codecov Report

Merging #23712 (dc08411) into master (e13b80a) will increase coverage by 0.04%.
The diff coverage is 100.00%.

❗ Current head dc08411 differs from pull request most recent head 611da97. Consider uploading reports for the commit 611da97 to get more accurate results

@@            Coverage Diff             @@
##           master   #23712      +/-   ##
==========================================
+ Coverage   68.33%   68.37%   +0.04%     
==========================================
  Files        1957     1957              
  Lines       75628    75549      -79     
  Branches     8225     8225              
==========================================
- Hits        51680    51658      -22     
+ Misses      21835    21778      -57     
  Partials     2113     2113

Flag	Coverage Δ
hive	`53.47% <66.66%> (+0.08%)`	⬆️
mysql	`79.00% <66.66%> (+0.08%)`	⬆️
postgres	`79.08% <66.66%> (+0.08%)`	⬆️
presto	`53.40% <66.66%> (+0.08%)`	⬆️
python	`82.94% <100.00%> (+0.11%)`	⬆️
sqlite	`77.61% <66.66%> (+0.07%)`	⬆️
unit	`53.81% <100.00%> (+0.36%)`	⬆️

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

Impacted Files	Coverage Δ
...rset-frontend/src/components/ReportModal/index.tsx	`79.03% <ø> (ø)`
...mponents/useExploreAdditionalActionsMenu/index.jsx	`60.22% <ø> (-0.45%)`	⬇️
...-frontend/src/features/alerts/AlertReportModal.tsx	`54.44% <ø> (ø)`
...-frontend/src/visualizations/presets/MainPreset.js	`100.00% <ø> (ø)`
superset/charts/post_processing.py	`92.59% <ø> (+3.23%)`	⬆️
superset/examples/birth_names.py	`70.00% <ø> (ø)`
...perset/migrations/shared/migrate_viz/processors.py	`100.00% <100.00%> (ø)`
superset/viz.py	`62.31% <100.00%> (+0.92%)`	⬆️

... and 2 files with indirect coverage changes

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

villebro · 2023-04-19T16:02:18Z

@michael-s-molina same comment as in #23741; please set 🔥 to class PivotTableViz(BaseViz) in viz.py!

john-bodley

Thanks for adding this. So happy to see the removal of legacy visualizations.

superset/migrations/shared/migrate_viz/base.py

superset/migrations/versions/2023-04-14_11-10_9ba2ce3086e5_migrate_pivot_table_v1_to_v2.py

superset/viz.py

kgabryje

Tested on my local environment, LGTM!

villebro

Very solid work, highly appreciate this! ❤️ LGTM!

villebro · 2023-04-25T11:15:16Z

superset-frontend/packages/superset-ui-demo/package.json

@@ -24,11 +11,24 @@
    "analysis",
    "data"
  ],
-  "license": "Apache-2.0",
+  "homepage": "https://github.com/apache-superset/superset-ui#readme",


bycatch: since that repo is archived, I wonder if we should just point these to the main repo?

rusackas

Thank you!!!

zhaoyongjie · 2023-05-24T21:47:20Z

superset/translations/ko/LC_MESSAGES/messages.po

Could we update all the translation files in a separated pull request?

It would be possible but the idea was to make the PR atomic by removing the all legacy-related artifacts.

zhaoyongjie · 2023-05-24T22:05:56Z

tests/unit_tests/conftest.py

+def with_feature_flags(**mock_feature_flags):
+    """
+    Use this decorator to mock feature flags in tests.integration_tests.
+
+    Usage:
+
+        class TestYourFeature(SupersetTestCase):
+
+            @with_feature_flags(YOUR_FEATURE=True)
+            def test_your_feature_enabled(self):
+                self.assertEqual(is_feature_enabled("YOUR_FEATURE"), True)
+
+            @with_feature_flags(YOUR_FEATURE=False)
+            def test_your_feature_disabled(self):
+                self.assertEqual(is_feature_enabled("YOUR_FEATURE"), False)
+    """
+


The decorator just mocked a single FF item, in order to support more than 1 FF item, I suggest to provide a DICT structure as a parameter.

Lets tackle this in a follow-up PR given that this is a copy/paste of the decorator found in the integration tests.

pull-request-size bot added the size/L label Apr 17, 2023

michael-s-molina force-pushed the migrate-pivot-table branch from 42296d0 to fb3a4ec Compare April 19, 2023 13:44

pull-request-size bot added size/XXL and removed size/L labels Apr 19, 2023

michael-s-molina requested review from rusackas, john-bodley, kgabryje and villebro April 19, 2023 13:46

michael-s-molina marked this pull request as ready for review April 19, 2023 13:46

michael-s-molina requested a review from a team as a code owner April 19, 2023 13:46

michael-s-molina added risk:breaking-change Issues or PRs that will introduce breaking changes v3.0 Label added by the release manager to track PRs to be included in the 3.0 branch labels Apr 19, 2023

michael-s-molina requested review from jinghua-qa, geido, eschutho and betodealmeida as code owners April 20, 2023 13:27

john-bodley reviewed Apr 20, 2023

View reviewed changes

superset/migrations/shared/migrate_viz/base.py Outdated Show resolved Hide resolved

superset/migrations/versions/2023-04-14_11-10_9ba2ce3086e5_migrate_pivot_table_v1_to_v2.py Outdated Show resolved Hide resolved

superset/viz.py Show resolved Hide resolved

michael-s-molina force-pushed the migrate-pivot-table branch from f7fb7c8 to dad6109 Compare April 21, 2023 20:11

kgabryje approved these changes Apr 25, 2023

View reviewed changes

villebro approved these changes Apr 25, 2023

View reviewed changes

rusackas approved these changes Apr 25, 2023

View reviewed changes

michael-s-molina force-pushed the migrate-pivot-table branch from 9187061 to 0f5a741 Compare April 25, 2023 20:06

michael-s-molina added the hold! On hold label Apr 26, 2023

michael-s-molina force-pushed the migrate-pivot-table branch from 0f5a741 to 98523e6 Compare May 2, 2023 13:04

michael-s-molina force-pushed the migrate-pivot-table branch 2 times, most recently from 8790a85 to 022d554 Compare May 23, 2023 19:32

zhaoyongjie reviewed May 24, 2023

View reviewed changes

michael-s-molina force-pushed the migrate-pivot-table branch 3 times, most recently from 184a21b to 2c86c6d Compare June 6, 2023 19:14

michael-s-molina added 15 commits June 8, 2023 08:57

feat: Migrates Pivot Table v1 to v2

a786a65

Additional fixes

2cdaf98

Removes the plugin

d7e0748

UPDATING.md

3a5c601

Removes PivotTableViz from viz.py

c4de75b

Removes unused imports

c3e97e1

Fixes integration tests

2b2817c

Fixes down revision

d3ce30c

Fixes source form data in unit test

7f67dcc

Updates JSON translation files

4076e98

Adjusts migration base

676420e

Fixes revision order

166327f

Fixes walrus lint error

063805b

Removes unused imports

8a0ba5e

Rebases

611da97

michael-s-molina force-pushed the migrate-pivot-table branch from 2c86c6d to 611da97 Compare June 8, 2023 12:14

michael-s-molina merged commit 522eb97 into apache:master Jun 8, 2023

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

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

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Migrates Pivot Table v1 to v2 #23712

feat: Migrates Pivot Table v1 to v2 #23712

michael-s-molina commented Apr 17, 2023 •

edited

Loading

michael-s-molina commented Apr 19, 2023 •

edited

Loading

codecov bot commented Apr 19, 2023 •

edited

Loading

villebro commented Apr 19, 2023

john-bodley left a comment

kgabryje left a comment

villebro left a comment

villebro Apr 25, 2023

rusackas left a comment

zhaoyongjie May 24, 2023

michael-s-molina May 25, 2023

zhaoyongjie May 24, 2023

michael-s-molina May 25, 2023

feat: Migrates Pivot Table v1 to v2 #23712

feat: Migrates Pivot Table v1 to v2 #23712

Conversation

michael-s-molina commented Apr 17, 2023 • edited Loading

SUMMARY

AFTER SCREENSHOT

TESTING INSTRUCTIONS

ADDITIONAL INFORMATION

michael-s-molina commented Apr 19, 2023 • edited Loading

codecov bot commented Apr 19, 2023 • edited Loading

Codecov Report

villebro commented Apr 19, 2023

john-bodley left a comment

Choose a reason for hiding this comment

kgabryje left a comment

Choose a reason for hiding this comment

villebro left a comment

Choose a reason for hiding this comment

villebro Apr 25, 2023

Choose a reason for hiding this comment

rusackas left a comment

Choose a reason for hiding this comment

zhaoyongjie May 24, 2023

Choose a reason for hiding this comment

michael-s-molina May 25, 2023

Choose a reason for hiding this comment

zhaoyongjie May 24, 2023

Choose a reason for hiding this comment

michael-s-molina May 25, 2023

Choose a reason for hiding this comment

michael-s-molina commented Apr 17, 2023 •

edited

Loading

michael-s-molina commented Apr 19, 2023 •

edited

Loading

codecov bot commented Apr 19, 2023 •

edited

Loading