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(tags): Export and Import Functionality for Superset Dashboards and Charts #30833

Open
wants to merge 29 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
9f34dd8
DR-4503: Added export Tags functionality for Superset Dashboards and …
Oct 25, 2024
a067536
Merge pull request #1 from asher-lab/DR-4503
asher-lab Oct 25, 2024
0978578
Merge branch 'apache:master' into master
asher-lab Oct 28, 2024
8b3fb08
Merge branch 'apache:master' into master
asher-lab Oct 31, 2024
9c289bd
DR-4503: Added Import Tags functionality for Superset Dashboards and …
Oct 31, 2024
aaf4c2a
DR-4503: Exclude owner tag in Superset export
Oct 31, 2024
2ba437d
Merge pull request #2 from asher-lab/DR-4503-import
asher-lab Oct 31, 2024
3225c3a
DR-4503: Fix Export Tag to ignore owner:
Oct 31, 2024
fa2bbe2
Merge pull request #3 from asher-lab/exportTagLogic
asher-lab Oct 31, 2024
1a32ca8
Merge branch 'apache:master' into master
asher-lab Oct 31, 2024
9c3f0c5
Merge branch 'apache:master' into master
asher-lab Nov 1, 2024
c797866
Merge branch 'apache:master' into master
asher-lab Nov 4, 2024
2d2d2b0
Revert docker env secret string
Nov 4, 2024
d4e294f
Merge pull request #4 from asher-lab/master-1
asher-lab Nov 4, 2024
f96aa88
Superset remove unwanted files
Nov 4, 2024
df3d310
Merge pull request #5 from asher-lab/master-1
asher-lab Nov 4, 2024
3ec09bc
Add superset logic to handle disable tagging system
Nov 5, 2024
dbc1955
Merge pull request #6 from asher-lab/DisableTaggingSystem
asher-lab Nov 5, 2024
ab9ed0f
Add Unit Testing for Tag Export and Import in Superset
Nov 6, 2024
8dea188
Merge pull request #7 from asher-lab/AddUnitTesting
asher-lab Nov 6, 2024
d5074c2
Fix Superset Integration for test_export_chart_command_no_related
Nov 7, 2024
27b3bc5
Change to tags and upfate filter by TagType
Nov 7, 2024
99fb8f1
Merge pull request #8 from asher-lab/FixIntegrationTestSuperset
asher-lab Nov 7, 2024
62bfcd2
Merge branch 'apache:master' into master
asher-lab Nov 12, 2024
e91ff15
Fix test pipeline (#12)
asher-lab Nov 13, 2024
094770e
feat(tags): adjust tag export behavior for charts when called from da…
asher-lab Nov 15, 2024
15fa3e5
feat(tag): Remove ExportTagsCommand from AssetsExport and Remove Opti…
asher-lab Nov 21, 2024
0d355d4
Fix merge conflict for import charts/dashboards
Nov 22, 2024
2959809
Merge branch 'master' into z3
asher-lab Nov 22, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion docker/.env
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,6 @@ MAPBOX_API_KEY=''

# Make sure you set this to a unique secure random value on production
SUPERSET_SECRET_KEY=TEST_NON_DEV_SECRET

ENABLE_PLAYWRIGHT=false
PUPPETEER_SKIP_CHROMIUM_DOWNLOAD=true
BUILD_SUPERSET_FRONTEND_IN_DOCKER=true
1 change: 1 addition & 0 deletions superset/charts/schemas.py
Original file line number Diff line number Diff line change
Expand Up @@ -1550,6 +1550,7 @@ class ImportV1ChartSchema(Schema):
dataset_uuid = fields.UUID(required=True)
is_managed_externally = fields.Boolean(allow_none=True, dump_default=False)
external_url = fields.String(allow_none=True)
tags = fields.List(fields.String(), allow_none=True)


class ChartCacheWarmUpRequestSchema(Schema):
Expand Down
26 changes: 26 additions & 0 deletions superset/commands/chart/export.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,10 +26,13 @@
from superset.daos.chart import ChartDAO
from superset.commands.dataset.export import ExportDatasetsCommand
from superset.commands.export.models import ExportModelsCommand
from superset.commands.tag.export import ExportTagsCommand
from superset.models.slice import Slice
from superset.tags.models import TagType
from superset.utils.dict_import_export import EXPORT_VERSION
from superset.utils.file import get_filename
from superset.utils import json
from superset.extensions import feature_flag_manager

logger = logging.getLogger(__name__)

Expand Down Expand Up @@ -71,9 +74,23 @@
if model.table:
payload["dataset_uuid"] = str(model.table.uuid)

# Fetch tags from the database if TAGGING_SYSTEM is enabled
if feature_flag_manager.is_feature_enabled("TAGGING_SYSTEM"):
tags = getattr(model, "tags", [])
payload["tags"] = [tag.name for tag in tags if tag.type == TagType.custom]

Check warning on line 80 in superset/commands/chart/export.py

View check run for this annotation

Codecov / codecov/patch

superset/commands/chart/export.py#L79-L80

Added lines #L79 - L80 were not covered by tests
file_content = yaml.safe_dump(payload, sort_keys=False)
return file_content

_include_tags: bool = True # Default to True

@classmethod
def disable_tag_export(cls) -> None:
cls._include_tags = False

@classmethod
def enable_tag_export(cls) -> None:
cls._include_tags = True

@staticmethod
def _export(
model: Slice, export_related: bool = True
Expand All @@ -85,3 +102,12 @@

if model.table and export_related:
yield from ExportDatasetsCommand([model.table.id]).run()

# Check if the calling class is ExportDashboardCommands
if (
export_related
and ExportChartsCommand._include_tags
and feature_flag_manager.is_feature_enabled("TAGGING_SYSTEM")
):
chart_id = model.id
yield from ExportTagsCommand().export(chart_ids=[chart_id])

Check warning on line 113 in superset/commands/chart/export.py

View check run for this annotation

Codecov / codecov/patch

superset/commands/chart/export.py#L112-L113

Added lines #L112 - L113 were not covered by tests
Comment on lines +106 to +113
Copy link
Member

Choose a reason for hiding this comment

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

Why only include tags for dashboard exports? Why not always include tags here?

Copy link
Author

Choose a reason for hiding this comment

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

    _include_tags: bool = True  # Default to True

    @classmethod
    def disable_tag_export(cls) -> None:
        cls._include_tags = False

    @classmethod
    def enable_tag_export(cls) -> None:
        cls._include_tags = True
        # Check if the calling class is ExportDashboardCommands
        if (
            export_related
            and ExportChartsCommand._include_tags
            and feature_flag_manager.is_feature_enabled("TAGGING_SYSTEM")
        ):
            chart_id = model.id
            yield from ExportTagsCommand().export(chart_ids=[chart_id])

^^ Tags are included only for charts exports here.

Our dashboard export has its own line of export:

        if export_related:
            chart_ids = [chart.id for chart in model.slices]
            dashboard_ids = model.id
            ExportChartsCommand.disable_tag_export()
            yield from ExportChartsCommand(chart_ids).run()
            ExportChartsCommand.enable_tag_export()
            if feature_flag_manager.is_feature_enabled("TAGGING_SYSTEM"):
                yield from ExportTagsCommand.export(
                    dashboard_ids=dashboard_ids, chart_ids=chart_ids
                )

22 changes: 20 additions & 2 deletions superset/commands/chart/importers/v1/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,23 +14,27 @@
# KIND, either express or implied. See the License for the
# specific language governing permissions and limitations
# under the License.
from __future__ import annotations

from typing import Any

from marshmallow import Schema
from sqlalchemy.orm import Session # noqa: F401

from superset import db
from superset.charts.schemas import ImportV1ChartSchema
from superset.commands.chart.exceptions import ChartImportError
from superset.commands.chart.importers.v1.utils import import_chart
from superset.commands.database.importers.v1.utils import import_database
from superset.commands.dataset.importers.v1.utils import import_dataset
from superset.commands.importers.v1 import ImportModelsCommand
from superset.commands.importers.v1.utils import import_tag
from superset.commands.utils import update_chart_config_dataset
from superset.connectors.sqla.models import SqlaTable
from superset.daos.chart import ChartDAO
from superset.databases.schemas import ImportV1DatabaseSchema
from superset.datasets.schemas import ImportV1DatasetSchema
from superset.extensions import feature_flag_manager


class ImportChartsCommand(ImportModelsCommand):
Expand All @@ -47,7 +51,13 @@
import_error = ChartImportError

@staticmethod
def _import(configs: dict[str, Any], overwrite: bool = False) -> None:
def _import(
configs: dict[str, Any],
overwrite: bool = False,
contents: dict[str, Any] | None = None,
) -> None:
if contents is None:
contents = {}

Check warning on line 60 in superset/commands/chart/importers/v1/__init__.py

View check run for this annotation

Codecov / codecov/patch

superset/commands/chart/importers/v1/__init__.py#L60

Added line #L60 was not covered by tests
# discover datasets associated with charts
dataset_uuids: set[str] = set()
for file_name, config in configs.items():
Expand Down Expand Up @@ -93,4 +103,12 @@
"datasource_name": dataset.table_name,
}
config = update_chart_config_dataset(config, dataset_dict)
import_chart(config, overwrite=overwrite)
chart = import_chart(config, overwrite=overwrite)

# Handle tags using import_tag function
if feature_flag_manager.is_feature_enabled("TAGGING_SYSTEM"):
if "tags" in config:
new_tag_names = config["tags"]
import_tag(

Check warning on line 112 in superset/commands/chart/importers/v1/__init__.py

View check run for this annotation

Codecov / codecov/patch

superset/commands/chart/importers/v1/__init__.py#L110-L112

Added lines #L110 - L112 were not covered by tests
new_tag_names, contents, chart.id, "chart", db.session
)
15 changes: 15 additions & 0 deletions superset/commands/dashboard/export.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
import yaml

from superset.commands.chart.export import ExportChartsCommand
from superset.commands.tag.export import ExportTagsCommand
from superset.commands.dashboard.exceptions import DashboardNotFoundError
from superset.commands.dashboard.importers.v1.utils import find_chart_uuids
from superset.daos.dashboard import DashboardDAO
Expand All @@ -33,9 +34,11 @@
from superset.daos.dataset import DatasetDAO
from superset.models.dashboard import Dashboard
from superset.models.slice import Slice
from superset.tags.models import TagType
from superset.utils.dict_import_export import EXPORT_VERSION
from superset.utils.file import get_filename
from superset.utils import json
from superset.extensions import feature_flag_manager # Import the feature flag manager

logger = logging.getLogger(__name__)

Expand Down Expand Up @@ -159,6 +162,11 @@

payload["version"] = EXPORT_VERSION

# Check if the TAGGING_SYSTEM feature is enabled
if feature_flag_manager.is_feature_enabled("TAGGING_SYSTEM"):
tags = model.tags if hasattr(model, "tags") else []
payload["tags"] = [tag.name for tag in tags if tag.type == TagType.custom]

Check warning on line 168 in superset/commands/dashboard/export.py

View check run for this annotation

Codecov / codecov/patch

superset/commands/dashboard/export.py#L167-L168

Added lines #L167 - L168 were not covered by tests

file_content = yaml.safe_dump(payload, sort_keys=False)
return file_content

Expand All @@ -173,7 +181,14 @@

if export_related:
chart_ids = [chart.id for chart in model.slices]
dashboard_ids = model.id
ExportChartsCommand.disable_tag_export()
yield from ExportChartsCommand(chart_ids).run()
ExportChartsCommand.enable_tag_export()
if feature_flag_manager.is_feature_enabled("TAGGING_SYSTEM"):
yield from ExportTagsCommand.export(

Check warning on line 189 in superset/commands/dashboard/export.py

View check run for this annotation

Codecov / codecov/patch

superset/commands/dashboard/export.py#L189

Added line #L189 was not covered by tests
dashboard_ids=dashboard_ids, chart_ids=chart_ids
)

payload = model.export_to_dict(
recursive=False,
Expand Down
34 changes: 32 additions & 2 deletions superset/commands/dashboard/importers/v1/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@
# specific language governing permissions and limitations
# under the License.

from __future__ import annotations

from typing import Any

from marshmallow import Schema
Expand All @@ -34,11 +36,13 @@
from superset.commands.database.importers.v1.utils import import_database
from superset.commands.dataset.importers.v1.utils import import_dataset
from superset.commands.importers.v1 import ImportModelsCommand
from superset.commands.importers.v1.utils import import_tag
from superset.commands.utils import update_chart_config_dataset
from superset.daos.dashboard import DashboardDAO
from superset.dashboards.schemas import ImportV1DashboardSchema
from superset.databases.schemas import ImportV1DatabaseSchema
from superset.datasets.schemas import ImportV1DatasetSchema
from superset.extensions import feature_flag_manager
from superset.migrations.shared.native_filters import migrate_dashboard
from superset.models.dashboard import Dashboard, dashboard_slices

Expand All @@ -58,9 +62,15 @@
import_error = DashboardImportError

# TODO (betodealmeida): refactor to use code from other commands
# pylint: disable=too-many-branches, too-many-locals
# pylint: disable=too-many-branches, too-many-locals, too-many-statements
@staticmethod
def _import(configs: dict[str, Any], overwrite: bool = False) -> None:
def _import(
configs: dict[str, Any],
overwrite: bool = False,
contents: dict[str, Any] | None = None,
) -> None:
if contents is None:
contents = {}

Check warning on line 73 in superset/commands/dashboard/importers/v1/__init__.py

View check run for this annotation

Codecov / codecov/patch

superset/commands/dashboard/importers/v1/__init__.py#L73

Added line #L73 was not covered by tests
# discover charts and datasets associated with dashboards
chart_uuids: set[str] = set()
dataset_uuids: set[str] = set()
Expand Down Expand Up @@ -120,6 +130,14 @@
charts.append(chart)
chart_ids[str(chart.uuid)] = chart.id

# Handle tags using import_tag function
if feature_flag_manager.is_feature_enabled("TAGGING_SYSTEM"):
if "tags" in config:
new_tag_names = config["tags"]
import_tag(

Check warning on line 137 in superset/commands/dashboard/importers/v1/__init__.py

View check run for this annotation

Codecov / codecov/patch

superset/commands/dashboard/importers/v1/__init__.py#L135-L137

Added lines #L135 - L137 were not covered by tests
new_tag_names, contents, chart.id, "chart", db.session
)

# store the existing relationship between dashboards and charts
existing_relationships = db.session.execute(
select([dashboard_slices.c.dashboard_id, dashboard_slices.c.slice_id])
Expand All @@ -140,6 +158,18 @@
if (dashboard.id, chart_id) not in existing_relationships:
dashboard_chart_ids.append((dashboard.id, chart_id))

# Handle tags using import_tag function
if feature_flag_manager.is_feature_enabled("TAGGING_SYSTEM"):
if "tags" in config:
new_tag_names = config["tags"]
import_tag(

Check warning on line 165 in superset/commands/dashboard/importers/v1/__init__.py

View check run for this annotation

Codecov / codecov/patch

superset/commands/dashboard/importers/v1/__init__.py#L163-L165

Added lines #L163 - L165 were not covered by tests
new_tag_names,
contents,
dashboard.id,
"dashboard",
db.session,
)

# set ref in the dashboard_slices table
values = [
{"dashboard_id": dashboard_id, "slice_id": chart_id}
Expand Down
8 changes: 6 additions & 2 deletions superset/commands/database/importers/v1/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
# specific language governing permissions and limitations
# under the License.

from typing import Any
from typing import Any, Optional

from marshmallow import Schema
from sqlalchemy.orm import Session # noqa: F401
Expand All @@ -42,7 +42,11 @@ class ImportDatabasesCommand(ImportModelsCommand):
import_error = DatabaseImportError

@staticmethod
def _import(configs: dict[str, Any], overwrite: bool = False) -> None:
def _import(
configs: dict[str, Any],
overwrite: bool = False,
contents: Optional[dict[str, Any]] = None,
) -> None:
# first import databases
database_ids: dict[str, int] = {}
for file_name, config in configs.items():
Expand Down
10 changes: 8 additions & 2 deletions superset/commands/dataset/importers/v1/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
# specific language governing permissions and limitations
# under the License.

from typing import Any
from typing import Any, Optional

from marshmallow import Schema
from sqlalchemy.orm import Session # noqa: F401
Expand All @@ -42,7 +42,13 @@
import_error = DatasetImportError

@staticmethod
def _import(configs: dict[str, Any], overwrite: bool = False) -> None:
def _import(
configs: dict[str, Any],
overwrite: bool = False,
contents: Optional[dict[str, Any]] = None,
) -> None:
if contents is None:
contents = {}

Check warning on line 51 in superset/commands/dataset/importers/v1/__init__.py

View check run for this annotation

Codecov / codecov/patch

superset/commands/dataset/importers/v1/__init__.py#L51

Added line #L51 was not covered by tests
# discover databases associated with datasets
database_uuids: set[str] = set()
for file_name, config in configs.items():
Expand Down
1 change: 1 addition & 0 deletions superset/commands/export/assets.py
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ def run(self) -> Iterator[tuple[str, Callable[[], str]]]:
ExportDashboardsCommand,
ExportSavedQueriesCommand,
]

for command in commands:
ids = [model.id for model in command.dao.find_all()]
for file_name, file_content in command(ids, export_related=False).run():
Expand Down
14 changes: 10 additions & 4 deletions superset/commands/importers/v1/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,9 @@
# KIND, either express or implied. See the License for the
# specific language governing permissions and limitations
# under the License.
from typing import Any, Optional
from __future__ import annotations

from typing import Any

from marshmallow import Schema, validate # noqa: F401
from marshmallow.exceptions import ValidationError
Expand Down Expand Up @@ -61,7 +63,11 @@ def __init__(self, contents: dict[str, str], *args: Any, **kwargs: Any):
self._configs: dict[str, Any] = {}

@staticmethod
def _import(configs: dict[str, Any], overwrite: bool = False) -> None:
def _import(
configs: dict[str, Any],
overwrite: bool = False,
contents: dict[str, Any] | None = None,
) -> None:
raise NotImplementedError("Subclasses MUST implement _import")

@classmethod
Expand All @@ -73,7 +79,7 @@ def run(self) -> None:
self.validate()

try:
self._import(self._configs, self.overwrite)
self._import(self._configs, self.overwrite, self.contents)
except CommandException:
raise
except Exception as ex:
Expand All @@ -84,7 +90,7 @@ def validate(self) -> None: # noqa: F811

# verify that the metadata file is present and valid
try:
metadata: Optional[dict[str, str]] = load_metadata(self.contents)
metadata: dict[str, str] | None = load_metadata(self.contents)
except ValidationError as exc:
exceptions.append(exc)
metadata = None
Expand Down
5 changes: 3 additions & 2 deletions superset/commands/importers/v1/examples.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
# KIND, either express or implied. See the License for the
# specific language governing permissions and limitations
# under the License.
from typing import Any
from typing import Any, Optional

Check warning on line 17 in superset/commands/importers/v1/examples.py

View check run for this annotation

Codecov / codecov/patch

superset/commands/importers/v1/examples.py#L17

Added line #L17 was not covered by tests

from marshmallow import Schema
from sqlalchemy.exc import MultipleResultsFound
Expand Down Expand Up @@ -90,6 +90,7 @@
def _import( # pylint: disable=too-many-locals, too-many-branches
configs: dict[str, Any],
overwrite: bool = False,
contents: Optional[dict[str, Any]] = None,
force_data: bool = False,
) -> None:
# import databases
Expand Down Expand Up @@ -129,7 +130,7 @@
dataset = import_dataset(
config,
overwrite=overwrite,
force_data=force_data,
force_data=bool(force_data),
ignore_permissions=True,
)
except MultipleResultsFound:
Expand Down
Loading
Loading