mirror of
https://github.com/apache/superset.git
synced 2026-09-05 15:01:44 +00:00
fix: Dashboard export with charts from multiple databases (#37120)
Co-authored-by: rusackas <evan@rusackas.com> Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
co-authored by
rusackas
Claude Opus 4.8
parent
fa59b44cfe
commit
b7301ac88a
@@ -91,8 +91,10 @@ class ExportChartsCommand(ExportModelsCommand):
|
||||
def enable_tag_export(cls) -> None:
|
||||
cls._include_tags = True
|
||||
|
||||
def run(self) -> Iterator[tuple[str, Callable[[], str]]]:
|
||||
yield from super().run()
|
||||
def run(
|
||||
self, seen: set[str] | None = None
|
||||
) -> Iterator[tuple[str, Callable[[], str]]]:
|
||||
yield from super().run(seen=seen)
|
||||
|
||||
# Tags are exported once for all requested charts (rather than per
|
||||
# chart in `_export`) so a multi-chart export doesn't lose tags to
|
||||
@@ -108,12 +110,17 @@ class ExportChartsCommand(ExportModelsCommand):
|
||||
|
||||
@staticmethod
|
||||
def _export(
|
||||
model: Slice, export_related: bool = True
|
||||
model: Slice, export_related: bool = True, seen: set[str] | None = None
|
||||
) -> Iterator[tuple[str, Callable[[], str]]]:
|
||||
# Initialize seen set if not provided
|
||||
if seen is None:
|
||||
seen = set()
|
||||
|
||||
yield (
|
||||
ExportChartsCommand._file_name(model),
|
||||
lambda: ExportChartsCommand._file_content(model),
|
||||
)
|
||||
|
||||
if model.table and export_related:
|
||||
yield from ExportDatasetsCommand([model.table.id]).run()
|
||||
# Pass the shared seen set to the dataset export command
|
||||
yield from ExportDatasetsCommand([model.table.id]).run(seen=seen)
|
||||
|
||||
@@ -383,8 +383,12 @@ class ExportDashboardsCommand(ExportModelsCommand):
|
||||
@staticmethod
|
||||
# ruff: noqa: C901
|
||||
def _export(
|
||||
model: Dashboard, export_related: bool = True
|
||||
model: Dashboard, export_related: bool = True, seen: set[str] | None = None
|
||||
) -> Iterator[tuple[str, Callable[[], str]]]:
|
||||
# Initialize seen set if not provided
|
||||
if seen is None:
|
||||
seen = set()
|
||||
|
||||
yield (
|
||||
ExportDashboardsCommand._file_name(model),
|
||||
lambda: ExportDashboardsCommand._file_content(model),
|
||||
@@ -395,8 +399,11 @@ class ExportDashboardsCommand(ExportModelsCommand):
|
||||
dashboard_ids = model.id
|
||||
command = ExportChartsCommand(chart_ids)
|
||||
command.disable_tag_export()
|
||||
yield from command.run()
|
||||
command.enable_tag_export()
|
||||
try:
|
||||
# Pass the shared seen set to the chart export command
|
||||
yield from command.run(seen=seen)
|
||||
finally:
|
||||
command.enable_tag_export()
|
||||
if feature_flag_manager.is_feature_enabled("TAGGING_SYSTEM"):
|
||||
yield from ExportTagsCommand(
|
||||
dashboard_ids=dashboard_ids, chart_ids=chart_ids
|
||||
@@ -406,7 +413,8 @@ class ExportDashboardsCommand(ExportModelsCommand):
|
||||
if model.theme:
|
||||
from superset.commands.theme.export import ExportThemesCommand
|
||||
|
||||
yield from ExportThemesCommand([model.theme.id]).run()
|
||||
# Pass the shared seen set to the theme export command
|
||||
yield from ExportThemesCommand([model.theme.id]).run(seen=seen)
|
||||
|
||||
payload = model.export_to_dict(
|
||||
recursive=False,
|
||||
@@ -435,7 +443,10 @@ class ExportDashboardsCommand(ExportModelsCommand):
|
||||
if dataset_id is not None:
|
||||
dataset = DatasetDAO.find_by_id(dataset_id)
|
||||
if dataset:
|
||||
yield from ExportDatasetsCommand([dataset_id]).run()
|
||||
# Pass the shared seen set to the dataset export command
|
||||
yield from ExportDatasetsCommand([dataset_id]).run(
|
||||
seen=seen
|
||||
)
|
||||
|
||||
# Export datasets referenced by display controls
|
||||
for customization in (
|
||||
@@ -446,4 +457,7 @@ class ExportDashboardsCommand(ExportModelsCommand):
|
||||
if dataset_id is not None:
|
||||
dataset = DatasetDAO.find_by_id(dataset_id)
|
||||
if dataset:
|
||||
yield from ExportDatasetsCommand([dataset_id]).run()
|
||||
# Pass the shared seen set to the dataset export command
|
||||
yield from ExportDatasetsCommand([dataset_id]).run(
|
||||
seen=seen
|
||||
)
|
||||
|
||||
@@ -113,8 +113,12 @@ class ExportDatabasesCommand(ExportModelsCommand):
|
||||
|
||||
@staticmethod
|
||||
def _export(
|
||||
model: Database, export_related: bool = True
|
||||
model: Database, export_related: bool = True, seen: set[str] | None = None
|
||||
) -> Iterator[tuple[str, Callable[[], str]]]:
|
||||
# Initialize seen set if not provided
|
||||
if seen is None:
|
||||
seen = set()
|
||||
|
||||
yield (
|
||||
ExportDatabasesCommand._file_name(model),
|
||||
lambda: ExportDatabasesCommand._file_content(model),
|
||||
|
||||
@@ -89,8 +89,12 @@ class ExportDatasetsCommand(ExportModelsCommand):
|
||||
|
||||
@staticmethod
|
||||
def _export(
|
||||
model: SqlaTable, export_related: bool = True
|
||||
model: SqlaTable, export_related: bool = True, seen: set[str] | None = None
|
||||
) -> Iterator[tuple[str, Callable[[], str]]]:
|
||||
# Initialize seen set if not provided
|
||||
if seen is None:
|
||||
seen = set()
|
||||
|
||||
yield (
|
||||
ExportDatasetsCommand._file_name(model),
|
||||
lambda: ExportDatasetsCommand._file_content(model),
|
||||
@@ -103,32 +107,41 @@ class ExportDatasetsCommand(ExportModelsCommand):
|
||||
)
|
||||
file_path = f"databases/{db_file_name}.yaml"
|
||||
|
||||
payload = model.database.export_to_dict(
|
||||
recursive=False,
|
||||
include_parent_ref=False,
|
||||
include_defaults=True,
|
||||
export_uuids=True,
|
||||
)
|
||||
# TODO (betodealmeida): move this logic to export_to_dict once this
|
||||
# becomes the default export endpoint
|
||||
if payload.get("extra"):
|
||||
try:
|
||||
payload["extra"] = json.loads(payload["extra"])
|
||||
except json.JSONDecodeError:
|
||||
logger.info("Unable to decode `extra` field: %s", payload["extra"])
|
||||
|
||||
if ssh_tunnel := model.database.ssh_tunnel:
|
||||
ssh_tunnel_payload = ssh_tunnel.export_to_dict(
|
||||
# Only yield the database file if not already seen. This is
|
||||
# critical to fix the issue where databases were being
|
||||
# duplicated and potentially overwritten when charts from
|
||||
# different databases were exported.
|
||||
if file_path not in seen:
|
||||
payload = model.database.export_to_dict(
|
||||
recursive=False,
|
||||
include_parent_ref=False,
|
||||
include_defaults=True,
|
||||
export_uuids=False,
|
||||
export_uuids=True,
|
||||
)
|
||||
payload["ssh_tunnel"] = mask_password_info(ssh_tunnel_payload)
|
||||
# TODO (betodealmeida): move this logic to export_to_dict once this
|
||||
# becomes the default export endpoint
|
||||
if payload.get("extra"):
|
||||
try:
|
||||
payload["extra"] = json.loads(payload["extra"])
|
||||
except json.JSONDecodeError:
|
||||
logger.info(
|
||||
"Unable to decode `extra` field: %s", payload["extra"]
|
||||
)
|
||||
|
||||
payload["version"] = EXPORT_VERSION
|
||||
if ssh_tunnel := model.database.ssh_tunnel:
|
||||
ssh_tunnel_payload = ssh_tunnel.export_to_dict(
|
||||
recursive=False,
|
||||
include_parent_ref=False,
|
||||
include_defaults=True,
|
||||
export_uuids=False,
|
||||
)
|
||||
payload["ssh_tunnel"] = mask_password_info(ssh_tunnel_payload)
|
||||
|
||||
yield (
|
||||
file_path,
|
||||
lambda: yaml.safe_dump(payload, sort_keys=False, allow_unicode=True),
|
||||
)
|
||||
payload["version"] = EXPORT_VERSION
|
||||
|
||||
yield (
|
||||
file_path,
|
||||
lambda: yaml.safe_dump(
|
||||
payload, sort_keys=False, allow_unicode=True
|
||||
),
|
||||
)
|
||||
|
||||
@@ -47,27 +47,45 @@ class ExportModelsCommand(BaseCommand):
|
||||
|
||||
@staticmethod
|
||||
def _file_content(model: Model) -> str:
|
||||
raise NotImplementedError("Subclasses MUST implement _export")
|
||||
raise NotImplementedError("Subclasses MUST implement _file_content")
|
||||
|
||||
@staticmethod
|
||||
def _export(
|
||||
model: Model, export_related: bool = True
|
||||
model: Model, export_related: bool = True, seen: set[str] | None = None
|
||||
) -> Iterator[tuple[str, Callable[[], str]]]:
|
||||
raise NotImplementedError("Subclasses MUST implement _export")
|
||||
|
||||
def run(self) -> Iterator[tuple[str, Callable[[], str]]]:
|
||||
def run(
|
||||
self, seen: set[str] | None = None
|
||||
) -> Iterator[tuple[str, Callable[[], str]]]:
|
||||
self.validate()
|
||||
|
||||
metadata = {
|
||||
"version": EXPORT_VERSION,
|
||||
"type": self.dao.model_cls.__name__, # type: ignore
|
||||
"timestamp": datetime.now(tz=timezone.utc).isoformat(),
|
||||
}
|
||||
yield METADATA_FILE_NAME, lambda: yaml.safe_dump(metadata, sort_keys=False)
|
||||
# Use provided seen set or create new one
|
||||
if seen is None:
|
||||
seen = set()
|
||||
should_add_metadata = True
|
||||
else:
|
||||
# If seen set is provided, we're being called from another command
|
||||
should_add_metadata = False
|
||||
|
||||
# Only add metadata if this is the root command
|
||||
if should_add_metadata:
|
||||
metadata = {
|
||||
"version": EXPORT_VERSION,
|
||||
"type": self.dao.model_cls.__name__, # type: ignore
|
||||
"timestamp": datetime.now(tz=timezone.utc).isoformat(),
|
||||
}
|
||||
if METADATA_FILE_NAME not in seen:
|
||||
yield (
|
||||
METADATA_FILE_NAME,
|
||||
lambda: yaml.safe_dump(metadata, sort_keys=False),
|
||||
)
|
||||
seen.add(METADATA_FILE_NAME)
|
||||
|
||||
seen = {METADATA_FILE_NAME}
|
||||
for model in self._models:
|
||||
for file_name, file_content in self._export(model, self.export_related):
|
||||
for file_name, file_content in self._export(
|
||||
model, self.export_related, seen
|
||||
):
|
||||
if file_name not in seen:
|
||||
yield file_name, file_content
|
||||
seen.add(file_name)
|
||||
|
||||
@@ -67,8 +67,12 @@ class ExportSavedQueriesCommand(ExportModelsCommand):
|
||||
|
||||
@staticmethod
|
||||
def _export(
|
||||
model: SavedQuery, export_related: bool = True
|
||||
model: SavedQuery, export_related: bool = True, seen: set[str] | None = None
|
||||
) -> Iterator[tuple[str, Callable[[], str]]]:
|
||||
# Initialize seen set if not provided
|
||||
if seen is None:
|
||||
seen = set()
|
||||
|
||||
yield (
|
||||
ExportSavedQueriesCommand._file_name(model),
|
||||
lambda: ExportSavedQueriesCommand._file_content(model),
|
||||
@@ -79,21 +83,25 @@ class ExportSavedQueriesCommand(ExportModelsCommand):
|
||||
database_slug = secure_filename(model.database.database_name)
|
||||
file_name = f"databases/{database_slug}.yaml"
|
||||
|
||||
payload = model.database.export_to_dict(
|
||||
recursive=False,
|
||||
include_parent_ref=False,
|
||||
include_defaults=True,
|
||||
export_uuids=True,
|
||||
)
|
||||
# TODO (betodealmeida): move this logic to export_to_dict once this
|
||||
# becomes the default export endpoint
|
||||
if "extra" in payload:
|
||||
try:
|
||||
payload["extra"] = json.loads(payload["extra"])
|
||||
except json.JSONDecodeError:
|
||||
logger.info("Unable to decode `extra` field: %s", payload["extra"])
|
||||
# Only yield if not already seen (similar to dataset export)
|
||||
if file_name not in seen:
|
||||
payload = model.database.export_to_dict(
|
||||
recursive=False,
|
||||
include_parent_ref=False,
|
||||
include_defaults=True,
|
||||
export_uuids=True,
|
||||
)
|
||||
# TODO (betodealmeida): move this logic to export_to_dict once this
|
||||
# becomes the default export endpoint
|
||||
if "extra" in payload:
|
||||
try:
|
||||
payload["extra"] = json.loads(payload["extra"])
|
||||
except (TypeError, json.JSONDecodeError):
|
||||
logger.info(
|
||||
"Unable to decode `extra` field: %s", payload["extra"]
|
||||
)
|
||||
|
||||
payload["version"] = EXPORT_VERSION
|
||||
payload["version"] = EXPORT_VERSION
|
||||
|
||||
file_content = yaml.safe_dump(payload, sort_keys=False)
|
||||
yield file_name, lambda: file_content
|
||||
file_content = yaml.safe_dump(payload, sort_keys=False)
|
||||
yield file_name, lambda: file_content
|
||||
|
||||
@@ -46,7 +46,9 @@ class ExportTagsCommand(ExportModelsCommand):
|
||||
self.dashboard_ids = dashboard_ids
|
||||
self.chart_ids = chart_ids
|
||||
|
||||
def run(self) -> Iterator[tuple[str, Callable[[], str]]]:
|
||||
def run(
|
||||
self, seen: set[str] | None = None
|
||||
) -> Iterator[tuple[str, Callable[[], str]]]:
|
||||
if not feature_flag_manager.is_feature_enabled("TAGGING_SYSTEM"):
|
||||
return
|
||||
|
||||
|
||||
@@ -67,8 +67,12 @@ class ExportThemesCommand(ExportModelsCommand):
|
||||
|
||||
@staticmethod
|
||||
def _export(
|
||||
model: Theme, export_related: bool = True
|
||||
model: Theme, export_related: bool = True, seen: set[str] | None = None
|
||||
) -> Iterator[tuple[str, Callable[[], str]]]:
|
||||
# Initialize seen set if not provided (for consistency)
|
||||
if seen is None:
|
||||
seen = set()
|
||||
|
||||
yield (
|
||||
ExportThemesCommand._file_name(model),
|
||||
lambda: ExportThemesCommand._file_content(model),
|
||||
|
||||
@@ -533,6 +533,110 @@ class TestExportDashboardsCommand(SupersetTestCase):
|
||||
{"dashboard_title": "World Bank's Data"},
|
||||
)
|
||||
|
||||
@pytest.mark.usefixtures("load_world_bank_dashboard_with_slices")
|
||||
@patch("superset.security.manager.g")
|
||||
@patch("superset.views.base.g")
|
||||
def test_export_dashboard_cross_database_charts(self, mock_g1, mock_g2):
|
||||
"""
|
||||
Test that dashboards with charts from multiple databases export correctly.
|
||||
This reproduces issue #37113 where charts from different databases were missing.
|
||||
"""
|
||||
mock_g1.user = security_manager.find_user("admin")
|
||||
mock_g2.user = security_manager.find_user("admin")
|
||||
|
||||
# Create a second database for testing
|
||||
second_db = Database(database_name="test_db_2", sqlalchemy_uri="sqlite://")
|
||||
db.session.add(second_db)
|
||||
|
||||
# Create a dataset in the second database
|
||||
second_dataset = SqlaTable(
|
||||
table_name="second_dataset",
|
||||
database=second_db,
|
||||
database_id=second_db.id,
|
||||
columns=[],
|
||||
)
|
||||
db.session.add(second_dataset)
|
||||
# Flush so `second_dataset.id` is populated before it's read below;
|
||||
# otherwise the chart would be constructed with `datasource_id=None`
|
||||
# and never actually link back to this dataset.
|
||||
db.session.flush()
|
||||
|
||||
# Create a chart using the second database's dataset
|
||||
chart_from_second_db = Slice(
|
||||
slice_name="Chart from Second Database",
|
||||
datasource_type="table",
|
||||
datasource_id=second_dataset.id,
|
||||
datasource_name=second_dataset.table_name,
|
||||
viz_type="bar",
|
||||
params=json.dumps({"viz_type": "bar"}),
|
||||
)
|
||||
db.session.add(chart_from_second_db)
|
||||
|
||||
# Get the example dashboard and add the new chart
|
||||
example_dashboard = (
|
||||
db.session.query(Dashboard).filter_by(slug="world_health").one()
|
||||
)
|
||||
|
||||
# Store original charts count
|
||||
original_charts_count = len(example_dashboard.slices)
|
||||
|
||||
# Add the new chart from different database to the dashboard
|
||||
example_dashboard.slices.append(chart_from_second_db)
|
||||
db.session.commit()
|
||||
|
||||
try:
|
||||
# Export the dashboard
|
||||
command = ExportDashboardsCommand([example_dashboard.id])
|
||||
contents = dict(command.run())
|
||||
|
||||
# Verify all databases are exported
|
||||
db_files = [key for key in contents.keys() if key.startswith("databases/")]
|
||||
assert len(db_files) >= 2, (
|
||||
f"Expected at least 2 database files, got {db_files}"
|
||||
)
|
||||
|
||||
# Verify the second database is included
|
||||
assert "databases/test_db_2.yaml" in contents.keys(), (
|
||||
f"Second database not found in export. Keys: {list(contents.keys())}"
|
||||
)
|
||||
|
||||
# Verify all charts are exported (original + new one)
|
||||
chart_files = [key for key in contents.keys() if key.startswith("charts/")]
|
||||
assert len(chart_files) == original_charts_count + 1, (
|
||||
f"Expected {original_charts_count + 1} charts, got {len(chart_files)}"
|
||||
)
|
||||
|
||||
# Verify the new chart from second database is included
|
||||
chart_from_second_db_file = None
|
||||
for key in chart_files:
|
||||
if f"Chart_from_Second_Database_{chart_from_second_db.id}" in key:
|
||||
chart_from_second_db_file = key
|
||||
break
|
||||
|
||||
assert chart_from_second_db_file is not None, (
|
||||
f"Chart from second database not found in export. "
|
||||
f"Chart files: {chart_files}"
|
||||
)
|
||||
|
||||
# Verify the dataset from second database is included
|
||||
dataset_files = [
|
||||
key for key in contents.keys() if key.startswith("datasets/")
|
||||
]
|
||||
second_dataset_file = (
|
||||
f"datasets/test_db_2/second_dataset_{second_dataset.id}.yaml"
|
||||
)
|
||||
assert second_dataset_file in contents.keys(), (
|
||||
f"Second dataset not found. Dataset files: {dataset_files}"
|
||||
)
|
||||
finally:
|
||||
# Clean up, even if an assertion above failed, so a failing run
|
||||
# doesn't leave extra Database/Slice/SqlaTable rows for later tests.
|
||||
example_dashboard.slices.remove(chart_from_second_db)
|
||||
db.session.delete(chart_from_second_db)
|
||||
db.session.delete(second_dataset)
|
||||
db.session.delete(second_db)
|
||||
db.session.commit()
|
||||
|
||||
|
||||
class TestImportDashboardsCommand(SupersetTestCase):
|
||||
def test_import_v0_dashboard_cli_export(self):
|
||||
|
||||
Reference in New Issue
Block a user