You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by be...@apache.org on 2023/07/17 20:52:54 UTC
[superset] branch migrate-charts-on-import updated: Skip the area migration
This is an automated email from the ASF dual-hosted git repository.
beto pushed a commit to branch migrate-charts-on-import
in repository https://gitbox.apache.org/repos/asf/superset.git
The following commit(s) were added to refs/heads/migrate-charts-on-import by this push:
new 30c4d966f8 Skip the area migration
30c4d966f8 is described below
commit 30c4d966f832c32f33545b3e944618f52a46db9c
Author: Beto Dealmeida <ro...@dealmeida.net>
AuthorDate: Mon Jul 17 13:51:43 2023 -0700
Skip the area migration
---
superset/charts/commands/importers/v1/utils.py | 6 +-
superset/migrations/shared/migrate_viz/base.py | 10 ++
.../migrations/shared/migrate_viz/processors.py | 8 +-
.../charts/commands/importers/v1/utils_test.py | 137 ++++++++++++++-------
4 files changed, 113 insertions(+), 48 deletions(-)
diff --git a/superset/charts/commands/importers/v1/utils.py b/superset/charts/commands/importers/v1/utils.py
index 079376fab6..e258abe269 100644
--- a/superset/charts/commands/importers/v1/utils.py
+++ b/superset/charts/commands/importers/v1/utils.py
@@ -26,7 +26,7 @@ from sqlalchemy.orm import Session
from superset import security_manager
from superset.commands.exceptions import ImportFailedError
from superset.migrations.shared.migrate_viz import processors
-from superset.migrations.shared.migrate_viz.base import MigrateViz
+from superset.migrations.shared.migrate_viz.base import ChartMigratorStatus, MigrateViz
from superset.models.slice import Slice
@@ -70,7 +70,9 @@ def migrate_chart(config: dict[str, Any]) -> dict[str, Any]:
migrators = {
class_.source_viz_type: class_
for class_ in processors.__dict__.values()
- if isclass(class_) and issubclass(class_, MigrateViz) and class_ != MigrateViz
+ if isclass(class_)
+ and issubclass(class_, MigrateViz)
+ and class_.status == ChartMigratorStatus.COMPLETE
}
output = copy.deepcopy(config)
diff --git a/superset/migrations/shared/migrate_viz/base.py b/superset/migrations/shared/migrate_viz/base.py
index e277fcd208..e10f56cac5 100644
--- a/superset/migrations/shared/migrate_viz/base.py
+++ b/superset/migrations/shared/migrate_viz/base.py
@@ -18,6 +18,7 @@ from __future__ import annotations
import copy
import json
+from enum import Enum
from typing import Any
from alembic import op
@@ -31,6 +32,12 @@ from superset.migrations.shared.utils import paginated_update, try_load_json
Base = declarative_base()
+class ChartMigratorStatus(Enum):
+ NOT_IMPLEMENTED = "NOT_IMPLEMENTED"
+ INCOMPLETE = "INCOMPLETE"
+ COMPLETE = "COMPLETE"
+
+
class Slice(Base): # type: ignore
__tablename__ = "slices"
@@ -51,6 +58,9 @@ class MigrateViz:
target_viz_type: str
has_x_axis_control: bool = False
+ # specific migrations need to override this
+ status = ChartMigratorStatus.NOT_IMPLEMENTED
+
def __init__(self, form_data: str) -> None:
self.data = try_load_json(form_data)
diff --git a/superset/migrations/shared/migrate_viz/processors.py b/superset/migrations/shared/migrate_viz/processors.py
index 4db308e04c..cb97e68504 100644
--- a/superset/migrations/shared/migrate_viz/processors.py
+++ b/superset/migrations/shared/migrate_viz/processors.py
@@ -16,7 +16,7 @@
# under the License.
from typing import Any
-from .base import MigrateViz
+from .base import ChartMigratorStatus, MigrateViz
class MigrateTreeMap(MigrateViz):
@@ -39,6 +39,8 @@ class MigrateAreaChart(MigrateViz):
target_viz_type = "echarts_area"
remove_keys = {"contribution", "stacked_style", "x_axis_label"}
+ status = ChartMigratorStatus.INCOMPLETE
+
def _pre_action(self) -> None:
if self.data.get("contribution"):
self.data["contributionMode"] = "row"
@@ -83,6 +85,8 @@ class MigratePivotTable(MigrateViz):
"var": "Sample Variance",
}
+ status = ChartMigratorStatus.COMPLETE
+
def _pre_action(self) -> None:
if pivot_margins := self.data.get("pivot_margins"):
self.data["colTotals"] = pivot_margins
@@ -104,6 +108,8 @@ class MigrateDualLine(MigrateViz):
}
remove_keys = {"metric", "metric_2"}
+ status = ChartMigratorStatus.COMPLETE
+
def _pre_action(self) -> None:
self.data["yAxisIndex"] = 0
self.data["yAxisIndexB"] = 1
diff --git a/tests/unit_tests/charts/commands/importers/v1/utils_test.py b/tests/unit_tests/charts/commands/importers/v1/utils_test.py
index ab8fd0d1f7..c99ecaf6d6 100644
--- a/tests/unit_tests/charts/commands/importers/v1/utils_test.py
+++ b/tests/unit_tests/charts/commands/importers/v1/utils_test.py
@@ -20,9 +20,11 @@ import json
from superset.charts.commands.importers.v1.utils import migrate_chart
-def test_migrate_chart() -> None:
+def test_migrate_chart_area() -> None:
"""
- Test the ``migrate_chart`` command when importing a chart.
+ Test the ``migrate_chart`` command when importing an area chart.
+
+ This is currently a no-op since the migration is not complete.
"""
chart_config = {
"slice_name": "Birth names by state",
@@ -32,32 +34,32 @@ def test_migrate_chart() -> None:
"viz_type": "area",
"params": json.dumps(
{
+ "adhoc_filters": [],
+ "annotation_layers": [],
+ "bottom_margin": "auto",
+ "color_scheme": "supersetColors",
+ "comparison_type": "values",
+ "dashboards": [],
"datasource": "21__table",
- "viz_type": "area",
+ "extra_form_data": {},
"granularity_sqla": "ds",
- "time_grain_sqla": "P1D",
- "time_range": "No filter",
- "metrics": ["count"],
- "adhoc_filters": [],
"groupby": ["state"],
+ "line_interpolation": "linear",
+ "metrics": ["count"],
"order_desc": True,
+ "rich_tooltip": True,
+ "rolling_type": "None",
"row_limit": 10000,
"show_brush": "auto",
"show_legend": True,
- "line_interpolation": "linear",
"stacked_style": "stack",
- "color_scheme": "supersetColors",
- "rich_tooltip": True,
- "bottom_margin": "auto",
- "x_ticks_layout": "auto",
+ "time_grain_sqla": "P1D",
+ "time_range": "No filter",
+ "viz_type": "area",
"x_axis_format": "smart_date",
- "y_axis_format": "SMART_NUMBER",
+ "x_ticks_layout": "auto",
"y_axis_bounds": [None, None],
- "rolling_type": "None",
- "comparison_type": "values",
- "annotation_layers": [],
- "extra_form_data": {},
- "dashboards": [],
+ "y_axis_format": "SMART_NUMBER",
}
),
"cache_timeout": None,
@@ -66,19 +68,85 @@ def test_migrate_chart() -> None:
"dataset_uuid": "a18b9cb0-b8d3-42ed-bd33-0f0fadbf0f6d",
}
+ new_config = migrate_chart(chart_config)
+ assert new_config == chart_config
+
+
+def test_migrate_pivot_table() -> None:
+ """
+ Test the ``migrate_chart`` command when importing an old pivot table.
+ """
+ chart_config = {
+ "slice_name": "Pivot Table",
+ "description": None,
+ "certified_by": None,
+ "certification_details": None,
+ "viz_type": "pivot_table",
+ "params": json.dumps(
+ {
+ "columns": ["state"],
+ "compare_lag": "10",
+ "compare_suffix": "o10Y",
+ "granularity_sqla": "ds",
+ "groupby": ["name"],
+ "limit": "25",
+ "markup_type": "markdown",
+ "metrics": [
+ {
+ "aggregate": "SUM",
+ "column": {
+ "column_name": "num",
+ "type": "BIGINT",
+ },
+ "expressionType": "SIMPLE",
+ "label": "Births",
+ "optionName": "metric_11",
+ },
+ ],
+ "row_limit": 50000,
+ "since": "100 years ago",
+ "time_range": "No filter",
+ "time_range_endpoints": ["inclusive", "exclusive"],
+ "until": "now",
+ "viz_type": "pivot_table",
+ },
+ ),
+ "cache_timeout": None,
+ "uuid": "ffd15af2-2188-425c-b6b4-df28aac45872",
+ "version": "1.0.0",
+ "dataset_uuid": "a18b9cb0-b8d3-42ed-bd33-0f0fadbf0f6d",
+ }
+
new_config = migrate_chart(chart_config)
assert new_config == {
- "slice_name": "Birth names by state",
+ "slice_name": "Pivot Table",
"description": None,
"certified_by": None,
"certification_details": None,
- "viz_type": "echarts_area",
+ "viz_type": "pivot_table_v2",
"params": json.dumps(
{
- "datasource": "21__table",
- "viz_type": "echarts_area",
- "time_grain_sqla": "P1D",
- "metrics": ["count"],
+ "groupbyColumns": ["state"],
+ "compare_lag": "10",
+ "compare_suffix": "o10Y",
+ "groupbyRows": ["name"],
+ "limit": "25",
+ "markup_type": "markdown",
+ "metrics": [
+ {
+ "aggregate": "SUM",
+ "column": {"column_name": "num", "type": "BIGINT"},
+ "expressionType": "SIMPLE",
+ "label": "Births",
+ "optionName": "metric_11",
+ }
+ ],
+ "series_limit": 50000,
+ "since": "100 years ago",
+ "time_range_endpoints": ["inclusive", "exclusive"],
+ "until": "now",
+ "viz_type": "pivot_table_v2",
+ "rowOrder": "value_z_to_a",
"adhoc_filters": [
{
"clause": "WHERE",
@@ -88,27 +156,6 @@ def test_migrate_chart() -> None:
"expressionType": "SIMPLE",
}
],
- "groupby": ["state"],
- "order_desc": True,
- "row_limit": 10000,
- "show_brush": "auto",
- "show_legend": True,
- "line_interpolation": "linear",
- "color_scheme": "supersetColors",
- "rich_tooltip": True,
- "bottom_margin": "auto",
- "x_ticks_layout": "auto",
- "x_axis_format": "smart_date",
- "y_axis_format": "SMART_NUMBER",
- "y_axis_bounds": [None, None],
- "rolling_type": "None",
- "comparison_type": "values",
- "annotation_layers": [],
- "extra_form_data": {},
- "dashboards": [],
- "show_extra_controls": True,
- "stack": "Stack",
- "x_axis": "ds",
}
),
"cache_timeout": None,