Skip to content

Commit

Permalink
fix(import-assets): Include all dashboard depedencies when using the …
Browse files Browse the repository at this point in the history 
…the --split flag
  • Loading branch information
@Vitor-Avila
Vitor-Avila committed May 9, 2024
1 parent 3908256 commit ca0b46d
Show file tree
Hide file tree
Showing 2 changed files with 163 additions and 3 deletions.
27 changes: 25 additions & 2 deletions src/preset_cli/cli/superset/sync/native/command.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
from io import BytesIO
from pathlib import Path
from types import ModuleType
from typing import Any, Dict, Iterator, Tuple
from typing import Any, Dict, Iterator, Set, Tuple
from zipfile import ZipFile

import backoff
Expand Down Expand Up @@ -272,7 +272,7 @@ def import_resources_individually(
("databases", lambda config: []),
("datasets", lambda config: [config["database_uuid"]]),
("charts", lambda config: [config["dataset_uuid"]]),
("dashboards", get_charts_uuids),
("dashboards", get_dashboard_related_uuids),
]
related_configs: Dict[str, Dict[Path, AssetConfig]] = {}
for resource_name, get_related_uuids in imports:
Expand All @@ -297,6 +297,16 @@ def import_resources_individually(
os.unlink(checkpoint_path)


def get_dashboard_related_uuids(config: AssetConfig) -> Iterator[str]:
"""
Extract dataset and chart UUID from a dashboard config.
"""
for uuid in get_charts_uuids(config):
yield uuid
for uuid in get_dataset_filter_uuids(config):
yield uuid


def get_charts_uuids(config: AssetConfig) -> Iterator[str]:
"""
Extract chart UUID from a dashboard config.
Expand All @@ -310,6 +320,19 @@ def get_charts_uuids(config: AssetConfig) -> Iterator[str]:
yield child["meta"]["uuid"]


def get_dataset_filter_uuids(config: AssetConfig) -> Set[str]:
"""
Extract dataset UUID for datasets that are used in dashboard filters.
"""
dataset_uuids = set()
for filter_config in config["metadata"].get("native_filter_configuration", []):
for target in filter_config["targets"]:
if uuid := target.get("datasetUuid"):
if uuid not in dataset_uuids:
dataset_uuids.add(uuid)
return dataset_uuids


def verify_db_connectivity(config: Dict[str, Any]) -> None:
"""
Test if we can connect to a given database.
Expand Down
139 changes: 138 additions & 1 deletion tests/cli/superset/sync/native/command_test.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -732,7 +732,10 @@ def test_verify_db_connectivity_error(mocker: MockerFixture) -> None:
)


def test_native_split(mocker: MockerFixture, fs: FakeFilesystem) -> None:
def test_native_split( # pylint: disable=too-many-locals
mocker: MockerFixture,
fs: FakeFilesystem,
) -> None:
"""
Test the ``native`` command with split imports.
"""
Expand Down Expand Up @@ -768,8 +771,84 @@ def test_native_split(mocker: MockerFixture, fs: FakeFilesystem) -> None:
},
},
},
"metadata": {},
"uuid": "4",
}
dataset_filter_config = {
"table_name": "filter_test",
"is_managed_externally": False,
"database_uuid": "1",
"uuid": "5",
}
dashboard_with_filter_config = {
"dashboard_title": "Some dashboard",
"is_managed_externally": False,
"position": {},
"metadata": {
"native_filter_configuration": [
{
"type": "NATIVE_FILTER",
"targets": [
{
"column": "some_column",
"datasetUuid": "5",
},
],
},
{
"type": "NATIVE_FILTER",
"targets": [
{
"column": "other_column",
"datasetUuid": "5",
},
],
},
{
"type": "NATIVE_FILTER",
"targets": [
{
"column": "blah",
"datasetUuid": "2",
},
],
},
],
},
"uuid": "6",
}
dashboard_with_temporal_filter = {
"dashboard_title": "Some dashboard",
"is_managed_externally": False,
"position": {},
"metadata": {
"native_filter_configuration": [
{
"type": "NATIVE_FILTER",
"targets": [],
},
],
},
"uuid": "6",
}
dashboard_deleted_dataset = {
"dashboard_title": "Some dashboard",
"is_managed_externally": False,
"position": {},
"metadata": {
"native_filter_configuration": [
{
"type": "NATIVE_FILTER",
"targets": [
{
"column": "some_column",
},
],
},
],
},
"uuid": "6",
}
fs.create_file(
root / "databases/gsheets.yaml",
contents=yaml.dump(database_config),
Expand All @@ -786,6 +865,22 @@ def test_native_split(mocker: MockerFixture, fs: FakeFilesystem) -> None:
root / "dashboards/dashboard.yaml",
contents=yaml.dump(dashboard_config),
)
fs.create_file(
root / "datasets/gsheets/filter_test.yaml",
contents=yaml.dump(dataset_filter_config),
)
fs.create_file(
root / "dashboards/dashboard_with_filter_config.yaml",
contents=yaml.dump(dashboard_with_filter_config),
)
fs.create_file(
root / "dashboards/dashboard_with_temporal_filter.yaml",
contents=yaml.dump(dashboard_with_temporal_filter),
)
fs.create_file(
root / "dashboards/dashboard_deleted_dataset.yaml",
contents=yaml.dump(dashboard_deleted_dataset),
)

SupersetClient = mocker.patch(
"preset_cli.cli.superset.sync.native.command.SupersetClient",
Expand All @@ -812,6 +907,16 @@ def test_native_split(mocker: MockerFixture, fs: FakeFilesystem) -> None:
client,
False,
),
mock.call(
{
"bundle/datasets/gsheets/filter_test.yaml": yaml.dump(
dataset_filter_config,
),
"bundle/databases/gsheets.yaml": yaml.dump(database_config),
},
client,
False,
),
mock.call(
{
"bundle/datasets/gsheets/test.yaml": yaml.dump(dataset_config),
Expand All @@ -829,6 +934,38 @@ def test_native_split(mocker: MockerFixture, fs: FakeFilesystem) -> None:
client,
False,
),
mock.call(
{
"bundle/dashboards/dashboard_deleted_dataset.yaml": yaml.dump(
dashboard_deleted_dataset,
),
},
client,
False,
),
mock.call(
{
"bundle/dashboards/dashboard_with_temporal_filter.yaml": yaml.dump(
dashboard_with_temporal_filter,
),
},
client,
False,
),
mock.call(
{
"bundle/dashboards/dashboard_with_filter_config.yaml": yaml.dump(
dashboard_with_filter_config,
),
"bundle/datasets/gsheets/filter_test.yaml": yaml.dump(
dataset_filter_config,
),
"bundle/datasets/gsheets/test.yaml": yaml.dump(dataset_config),
"bundle/databases/gsheets.yaml": yaml.dump(database_config),
},
client,
False,
),
mock.call(
{
"bundle/dashboards/dashboard.yaml": yaml.dump(dashboard_config),
Expand Down

0 comments on commit ca0b46d

Please sign in to comment.