Skip to content

Commit

Permalink
Addresses comments
Browse files Browse the repository at this point in the history
  • Loading branch information
michael-s-molina committed Jun 25, 2024
1 parent 2fe65a8 commit 2292f18
Show file tree
Hide file tree
Showing 2 changed files with 103 additions and 63 deletions.
152 changes: 100 additions & 52 deletions superset/cli/viz_migrations.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,31 @@
from __future__ import annotations

Check warning on line 17 in superset/cli/viz_migrations.py

View check run for this annotation

Codecov / codecov/patch

superset/cli/viz_migrations.py#L17

Added line #L17 was not covered by tests

from enum import Enum
from typing import Type

Check warning on line 20 in superset/cli/viz_migrations.py

View check run for this annotation

Codecov / codecov/patch

superset/cli/viz_migrations.py#L20

Added line #L20 was not covered by tests

import click
from click_option_group import optgroup, RequiredAnyOptionGroup

Check warning on line 23 in superset/cli/viz_migrations.py

View check run for this annotation

Codecov / codecov/patch

superset/cli/viz_migrations.py#L23

Added line #L23 was not covered by tests
from flask.cli import with_appcontext

from superset import db
from superset.migrations.shared.migrate_viz.base import (

Check warning on line 27 in superset/cli/viz_migrations.py

View check run for this annotation

Codecov / codecov/patch

superset/cli/viz_migrations.py#L27

Added line #L27 was not covered by tests
MigrateViz,
Slice,
)
from superset.migrations.shared.migrate_viz.processors import (

Check warning on line 31 in superset/cli/viz_migrations.py

View check run for this annotation

Codecov / codecov/patch

superset/cli/viz_migrations.py#L31

Added line #L31 was not covered by tests
MigrateAreaChart,
MigrateBarChart,
MigrateBubbleChart,
MigrateDistBarChart,
MigrateDualLine,
MigrateHeatmapChart,
MigrateHistogramChart,
MigrateLineChart,
MigratePivotTable,
MigrateSunburst,
MigrateTreeMap,
)
from superset.migrations.shared.utils import paginated_update

Check warning on line 44 in superset/cli/viz_migrations.py

View check run for this annotation

Codecov / codecov/patch

superset/cli/viz_migrations.py#L44

Added line #L44 was not covered by tests


class VizType(str, Enum):
Expand All @@ -38,6 +58,25 @@ class VizType(str, Enum):
TREEMAP = "treemap"


MIGRATIONS: dict[VizType, Type[MigrateViz]] = {

Check warning on line 61 in superset/cli/viz_migrations.py

View check run for this annotation

Codecov / codecov/patch

superset/cli/viz_migrations.py#L61

Added line #L61 was not covered by tests
VizType.AREA: MigrateAreaChart,
VizType.BAR: MigrateBarChart,
VizType.BUBBLE: MigrateBubbleChart,
VizType.DIST_BAR: MigrateDistBarChart,
VizType.DUAL_LINE: MigrateDualLine,
VizType.HEATMAP: MigrateHeatmapChart,
VizType.HISTOGRAM: MigrateHistogramChart,
VizType.LINE: MigrateLineChart,
VizType.PIVOT_TABLE: MigratePivotTable,
VizType.SUNBURST: MigrateSunburst,
VizType.TREEMAP: MigrateTreeMap,
}

PREVIOUS_VERSION = {

Check warning on line 75 in superset/cli/viz_migrations.py

View check run for this annotation

Codecov / codecov/patch

superset/cli/viz_migrations.py#L75

Added line #L75 was not covered by tests
migration.target_viz_type: migration for migration in MIGRATIONS.values()
}


@click.group()
def migrate_viz() -> None:
"""
Expand All @@ -47,73 +86,82 @@ def migrate_viz() -> None:

@migrate_viz.command()
@with_appcontext
@click.option(
@optgroup.group(
cls=RequiredAnyOptionGroup,
)
@optgroup.option(
"--viz_type",
"-t",
help=f"The viz type to upgrade: {', '.join(list(VizType))}",
required=True,
type=str,
)
@click.option(
"--chart_id",
help="The chart ID to upgrade",
type=int,
@optgroup.option(

Check warning on line 98 in superset/cli/viz_migrations.py

View check run for this annotation

Codecov / codecov/patch

superset/cli/viz_migrations.py#L98

Added line #L98 was not covered by tests
"-ids",
help="A comma separated list of chart IDs to upgrade",
type=str,
)
def upgrade(viz_type: str, chart_id: int | None = None) -> None:
def upgrade(viz_type: str, ids: str | None = None) -> None:

Check warning on line 103 in superset/cli/viz_migrations.py

View check run for this annotation

Codecov / codecov/patch

superset/cli/viz_migrations.py#L103

Added line #L103 was not covered by tests
"""Upgrade a viz to the latest version."""
migrate(VizType(viz_type), chart_id)
if ids is None:
migrate_by_viz_type(VizType(viz_type))

Check warning on line 106 in superset/cli/viz_migrations.py

View check run for this annotation

Codecov / codecov/patch

superset/cli/viz_migrations.py#L105-L106

Added lines #L105 - L106 were not covered by tests
else:
migrate_by_ids(ids)

Check warning on line 108 in superset/cli/viz_migrations.py

View check run for this annotation

Codecov / codecov/patch

superset/cli/viz_migrations.py#L108

Added line #L108 was not covered by tests


@migrate_viz.command()
@with_appcontext
@click.option(
@optgroup.group(
cls=RequiredAnyOptionGroup,
)
@optgroup.option(
"--viz_type",
"-t",
help=f"The viz type to downgrade: {', '.join(list(VizType))}",
required=True,
type=str,
)
@click.option(
"--chart_id",
help="The chart ID to downgrade",
type=int,
@optgroup.option(

Check warning on line 122 in superset/cli/viz_migrations.py

View check run for this annotation

Codecov / codecov/patch

superset/cli/viz_migrations.py#L122

Added line #L122 was not covered by tests
"-ids",
help="A comma separated list of chart IDs to downgrade",
type=str,
)
def downgrade(viz_type: str, chart_id: int | None = None) -> None:
def downgrade(viz_type: str, ids: str | None = None) -> None:

Check warning on line 127 in superset/cli/viz_migrations.py

View check run for this annotation

Codecov / codecov/patch

superset/cli/viz_migrations.py#L127

Added line #L127 was not covered by tests
"""Downgrade a viz to the previous version."""
migrate(VizType(viz_type), chart_id, is_downgrade=True)


def migrate(
viz_type: VizType, chart_id: int | None = None, is_downgrade: bool = False
) -> None:
"""Migrate a viz from one type to another."""
# pylint: disable=import-outside-toplevel
from superset.migrations.shared.migrate_viz.processors import (
MigrateAreaChart,
MigrateBarChart,
MigrateBubbleChart,
MigrateDistBarChart,
MigrateDualLine,
MigrateHeatmapChart,
MigrateHistogramChart,
MigrateLineChart,
MigratePivotTable,
MigrateSunburst,
MigrateTreeMap,
)

migrations = {
VizType.AREA: MigrateAreaChart,
VizType.BAR: MigrateBarChart,
VizType.BUBBLE: MigrateBubbleChart,
VizType.DIST_BAR: MigrateDistBarChart,
VizType.DUAL_LINE: MigrateDualLine,
VizType.HEATMAP: MigrateHeatmapChart,
VizType.HISTOGRAM: MigrateHistogramChart,
VizType.LINE: MigrateLineChart,
VizType.PIVOT_TABLE: MigratePivotTable,
VizType.SUNBURST: MigrateSunburst,
VizType.TREEMAP: MigrateTreeMap,
}
if ids is None:
migrate_by_viz_type(VizType(viz_type), is_downgrade=True)

Check warning on line 130 in superset/cli/viz_migrations.py

View check run for this annotation

Codecov / codecov/patch

superset/cli/viz_migrations.py#L129-L130

Added lines #L129 - L130 were not covered by tests
else:
migrate_by_ids(ids, is_downgrade=True)

Check warning on line 132 in superset/cli/viz_migrations.py

View check run for this annotation

Codecov / codecov/patch

superset/cli/viz_migrations.py#L132

Added line #L132 was not covered by tests


def migrate_by_viz_type(viz_type: VizType, is_downgrade: bool = False) -> None:

Check warning on line 135 in superset/cli/viz_migrations.py

View check run for this annotation

Codecov / codecov/patch

superset/cli/viz_migrations.py#L135

Added line #L135 was not covered by tests
"""
Migrate all charts of a viz type.
:param viz_type: The viz type to migrate
:param is_downgrade: Whether to downgrade the charts. Default is upgrade.
"""
migration: Type[MigrateViz] = MIGRATIONS[viz_type]

Check warning on line 142 in superset/cli/viz_migrations.py

View check run for this annotation

Codecov / codecov/patch

superset/cli/viz_migrations.py#L142

Added line #L142 was not covered by tests
if is_downgrade:
migrations[viz_type].downgrade(db.session, chart_id)
migration.downgrade(db.session)

Check warning on line 144 in superset/cli/viz_migrations.py

View check run for this annotation

Codecov / codecov/patch

superset/cli/viz_migrations.py#L144

Added line #L144 was not covered by tests
else:
migrations[viz_type].upgrade(db.session, chart_id)
migration.upgrade(db.session)

Check warning on line 146 in superset/cli/viz_migrations.py

View check run for this annotation

Codecov / codecov/patch

superset/cli/viz_migrations.py#L146

Added line #L146 was not covered by tests


def migrate_by_ids(ids: str, is_downgrade: bool = False) -> None:

Check warning on line 149 in superset/cli/viz_migrations.py

View check run for this annotation

Codecov / codecov/patch

superset/cli/viz_migrations.py#L149

Added line #L149 was not covered by tests
"""
Migrate a subset of charts by a list of IDs.
:param ids: List of chart IDs to migrate
:param is_downgrade: Whether to downgrade the charts. Default is upgrade.
"""
id_list = [int(i) for i in ids.split(",")]
slices = db.session.query(Slice).filter(Slice.id.in_(id_list))
for slc in paginated_update(

Check warning on line 158 in superset/cli/viz_migrations.py

View check run for this annotation

Codecov / codecov/patch

superset/cli/viz_migrations.py#L156-L158

Added lines #L156 - L158 were not covered by tests
slices,
lambda current, total: print(
f"{('Downgraded' if is_downgrade else 'Upgraded')} {current}/{total} charts"
),
):
if is_downgrade:
PREVIOUS_VERSION[slc.viz_type].downgrade_slice(slc)

Check warning on line 165 in superset/cli/viz_migrations.py

View check run for this annotation

Codecov / codecov/patch

superset/cli/viz_migrations.py#L164-L165

Added lines #L164 - L165 were not covered by tests
else:
MIGRATIONS[slc.viz_type].upgrade_slice(slc)

Check warning on line 167 in superset/cli/viz_migrations.py

View check run for this annotation

Codecov / codecov/patch

superset/cli/viz_migrations.py#L167

Added line #L167 was not covered by tests
14 changes: 3 additions & 11 deletions superset/migrations/shared/migrate_viz/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,6 @@
# KIND, either express or implied. See the License for the
# specific language governing permissions and limitations
# under the License.
from __future__ import annotations

import copy
from typing import Any

Expand Down Expand Up @@ -153,26 +151,20 @@ def downgrade_slice(cls, slc: Slice) -> None:
slc.query_context = json.dumps(query_context)

@classmethod
def upgrade(cls, session: Session, chart_id: int | None = None) -> None:
slices = session.query(Slice).filter(
and_(
Slice.viz_type == cls.source_viz_type,
Slice.id == chart_id if chart_id is not None else True,
)
)
def upgrade(cls, session: Session) -> None:
slices = session.query(Slice).filter(Slice.viz_type == cls.source_viz_type)
for slc in paginated_update(
slices,
lambda current, total: print(f"Upgraded {current}/{total} charts"),
):
cls.upgrade_slice(slc)

@classmethod
def downgrade(cls, session: Session, chart_id: int | None = None) -> None:
def downgrade(cls, session: Session) -> None:
slices = session.query(Slice).filter(
and_(
Slice.viz_type == cls.target_viz_type,
Slice.params.like(f"%{FORM_DATA_BAK_FIELD_NAME}%"),
Slice.id == chart_id if chart_id is not None else True,
)
)
for slc in paginated_update(
Expand Down

0 comments on commit 2292f18

Please sign in to comment.