L4XB commented on code in PR #44347:
URL: https://github.com/apache/superset/pull/44347#discussion_r4333879260
##########
superset/commands/export/assets.py:
##########
@@ -77,11 +85,69 @@ def run(self) -> Iterator[tuple[str, Callable[[], str]]]:
dashboard_ids = ids
elif command == ExportChartsCommand:
chart_ids = ids
+ elif command == ExportDatasetsCommand:
+ dataset_ids = ids
+
+ yield from self._export_referenced_trashed_datasets(
+ chart_ids, dashboard_ids, dataset_ids, seen
+ )
yield from ExportTagsCommand(
dashboard_ids=dashboard_ids,
chart_ids=chart_ids,
).run()
+ @classmethod
+ def _export_referenced_trashed_datasets(
+ cls,
+ chart_ids: list[int | str],
+ dashboard_ids: list[int | str],
+ dataset_ids: list[int | str],
+ seen: set[str],
+ ) -> Iterator[tuple[str, Callable[[], str]]]:
+ if trashed_ids := cls._referenced_trashed_dataset_ids(
+ chart_ids, dashboard_ids, dataset_ids
+ ):
+ for file_name, file_content in ExportDatasetsCommand(
+ trashed_ids, export_related=False, include_deleted=True
+ ).run():
+ if file_name not in seen:
+ yield file_name, file_content
+ seen.add(file_name)
+
+ @staticmethod
+ def _referenced_trashed_dataset_ids(
+ chart_ids: list[int | str],
+ dashboard_ids: list[int | str],
+ dataset_ids: list[int | str],
+ ) -> list[int]:
+ """Datasets in the trash that the exported charts and native filters
name.
+
+ Datasets are exported from the visibility-filtered ``find_all()``,
which
+ leaves the trash out. A chart or a native filter whose dataset is in
the
+ trash still writes its ``dataset_uuid`` / ``datasetUuid``, and the
import
+ resolves those from the bundle, so those datasets are exported as well.
+ The DAO base filter still applies to every lookup.
+ """
+ referenced = {
+ dataset.id
+ for chart in ChartDAO.find_by_ids(chart_ids)
+ if (dataset := find_chart_dataset(chart)) is not None
Review Comment:
Fixed in 401370416f, with the first option: chart ids and native-filter ids
now go through one `DatasetDAO.find_by_ids(..., skip_visibility_filter=True)`
lookup, so the base filter decides for both. The chart side takes
`datasource_id` for table-type charts instead of `Slice.table`. A live dataset
the user cannot read is therefore dropped and never reaches
`ExportDatasetsCommand`.
`test_assets_export_leaves_out_a_live_dataset_the_user_cannot_read` (with
`chart_editor_without_dataset_access`) expects no ids, and it got the live
dataset back on the previous head.
##########
superset/commands/export/assets.py:
##########
@@ -77,11 +85,69 @@ def run(self) -> Iterator[tuple[str, Callable[[], str]]]:
dashboard_ids = ids
elif command == ExportChartsCommand:
chart_ids = ids
+ elif command == ExportDatasetsCommand:
+ dataset_ids = ids
+
+ yield from self._export_referenced_trashed_datasets(
+ chart_ids, dashboard_ids, dataset_ids, seen
+ )
yield from ExportTagsCommand(
dashboard_ids=dashboard_ids,
chart_ids=chart_ids,
).run()
+ @classmethod
+ def _export_referenced_trashed_datasets(
+ cls,
+ chart_ids: list[int | str],
+ dashboard_ids: list[int | str],
+ dataset_ids: list[int | str],
+ seen: set[str],
+ ) -> Iterator[tuple[str, Callable[[], str]]]:
+ if trashed_ids := cls._referenced_trashed_dataset_ids(
+ chart_ids, dashboard_ids, dataset_ids
+ ):
+ for file_name, file_content in ExportDatasetsCommand(
+ trashed_ids, export_related=False, include_deleted=True
+ ).run():
+ if file_name not in seen:
+ yield file_name, file_content
+ seen.add(file_name)
+
+ @staticmethod
+ def _referenced_trashed_dataset_ids(
+ chart_ids: list[int | str],
+ dashboard_ids: list[int | str],
+ dataset_ids: list[int | str],
+ ) -> list[int]:
+ """Datasets in the trash that the exported charts and native filters
name.
+
+ Datasets are exported from the visibility-filtered ``find_all()``,
which
+ leaves the trash out. A chart or a native filter whose dataset is in
the
+ trash still writes its ``dataset_uuid`` / ``datasetUuid``, and the
import
+ resolves those from the bundle, so those datasets are exported as well.
+ The DAO base filter still applies to every lookup.
+ """
+ referenced = {
+ dataset.id
+ for chart in ChartDAO.find_by_ids(chart_ids)
+ if (dataset := find_chart_dataset(chart)) is not None
+ }
+ filter_ids: set[int] = set()
+ for dashboard in DashboardDAO.find_by_ids(dashboard_ids):
+ try:
+ metadata = json.loads(dashboard.json_metadata or "{}")
Review Comment:
Fixed in 401370416f: a parsed `json_metadata` that is not a dict names no
targets and is skipped, as `payload.get("metadata") or {}` effectively does in
the dashboard exporter.
`test_assets_export_reads_dashboards_whose_metadata_is_not_an_object` puts
`null` and `[]` dashboards next to one whose native filter targets a trashed
dataset and expects that dataset. On the previous head it raised
`AttributeError`.
##########
superset/commands/export/assets.py:
##########
@@ -77,11 +85,69 @@ def run(self) -> Iterator[tuple[str, Callable[[], str]]]:
dashboard_ids = ids
elif command == ExportChartsCommand:
chart_ids = ids
+ elif command == ExportDatasetsCommand:
+ dataset_ids = ids
+
+ yield from self._export_referenced_trashed_datasets(
+ chart_ids, dashboard_ids, dataset_ids, seen
+ )
yield from ExportTagsCommand(
dashboard_ids=dashboard_ids,
chart_ids=chart_ids,
).run()
+ @classmethod
+ def _export_referenced_trashed_datasets(
+ cls,
+ chart_ids: list[int | str],
+ dashboard_ids: list[int | str],
+ dataset_ids: list[int | str],
+ seen: set[str],
+ ) -> Iterator[tuple[str, Callable[[], str]]]:
+ if trashed_ids := cls._referenced_trashed_dataset_ids(
+ chart_ids, dashboard_ids, dataset_ids
+ ):
+ for file_name, file_content in ExportDatasetsCommand(
+ trashed_ids, export_related=False, include_deleted=True
+ ).run():
+ if file_name not in seen:
+ yield file_name, file_content
+ seen.add(file_name)
+
+ @staticmethod
+ def _referenced_trashed_dataset_ids(
+ chart_ids: list[int | str],
+ dashboard_ids: list[int | str],
+ dataset_ids: list[int | str],
+ ) -> list[int]:
+ """Datasets in the trash that the exported charts and native filters
name.
+
+ Datasets are exported from the visibility-filtered ``find_all()``,
which
+ leaves the trash out. A chart or a native filter whose dataset is in
the
+ trash still writes its ``dataset_uuid`` / ``datasetUuid``, and the
import
+ resolves those from the bundle, so those datasets are exported as well.
+ The DAO base filter still applies to every lookup.
+ """
+ referenced = {
+ dataset.id
+ for chart in ChartDAO.find_by_ids(chart_ids)
+ if (dataset := find_chart_dataset(chart)) is not None
+ }
+ filter_ids: set[int] = set()
+ for dashboard in DashboardDAO.find_by_ids(dashboard_ids):
+ try:
+ metadata = json.loads(dashboard.json_metadata or "{}")
+ except (TypeError, json.JSONDecodeError):
+ continue
+ filter_ids |= dashboard_dataset_ids(metadata)
Review Comment:
Fixed in 401370416f inside the shared helper: `dashboard_dataset_ids` now
only reads targets whose `datasourceType` is absent or `"table"`, the same
split `export_dashboard_references` makes. So the assets path and the dashboard
exporter normalise the same way. In `ExportDashboardsCommand` the semantic
targets have already been rewritten at that point, so nothing changes there.
`test_assets_export_does_not_read_a_semantic_view_target_as_a_table` gives a
filter target the trashed table's id and is parametrized over `semantic_view`,
`table` and no type. Only the semantic case leaves the table out, and that case
failed on the previous head.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
