rusackas commented on code in PR #42339:
URL: https://github.com/apache/superset/pull/42339#discussion_r3772051557
##########
superset/commands/chart/export.py:
##########
@@ -110,4 +110,4 @@ def _export(
and feature_flag_manager.is_feature_enabled("TAGGING_SYSTEM")
):
chart_id = model.id
- yield from ExportTagsCommand().export(chart_ids=[chart_id])
+ yield from ExportTagsCommand(chart_ids=[chart_id]).run()
Review Comment:
Good catch, this is real. Multi-chart export yielded a tags.yaml per chart
from inside _export(), but the parent's file-name de-dup only keeps the first,
so later charts' tags got dropped on a direct multi-chart export. Moved the tag
export out of _export() into an overridden run() that aggregates all requested
chart ids into one ExportTagsCommand call. Pushed in e924e37.
##########
superset/commands/tag/export.py:
##########
@@ -23,33 +23,78 @@
import yaml
from superset.daos.chart import ChartDAO
from superset.daos.dashboard import DashboardDAO
+from superset.daos.tag import TagDAO
from superset.extensions import feature_flag_manager
-from superset.tags.models import TagType
+from superset.tags.models import ObjectType, TagType
+from superset.commands.export.models import ExportModelsCommand
from superset.commands.tag.exceptions import TagNotFoundError
-# pylint: disable=too-few-public-methods
-class ExportTagsCommand:
+class ExportTagsCommand(ExportModelsCommand):
Review Comment:
Checked, there's no caller left using the old .export() static method.
assets.py, dashboard/export.py, and chart/export.py were all updated in this PR
to use ExportTagsCommand(...).run() instead, and the tests match that. Should
be safe to drop.
##########
superset/commands/tag/export.py:
##########
@@ -23,33 +23,78 @@
import yaml
from superset.daos.chart import ChartDAO
from superset.daos.dashboard import DashboardDAO
+from superset.daos.tag import TagDAO
from superset.extensions import feature_flag_manager
-from superset.tags.models import TagType
+from superset.tags.models import ObjectType, TagType
+from superset.commands.export.models import ExportModelsCommand
from superset.commands.tag.exceptions import TagNotFoundError
-# pylint: disable=too-few-public-methods
-class ExportTagsCommand:
+class ExportTagsCommand(ExportModelsCommand):
+ dao = TagDAO
not_found = TagNotFoundError
+ def __init__(
+ self,
+ model_ids: Optional[list[int]] = None,
+ export_related: bool = True,
+ *,
+ dashboard_ids: Optional[Union[int, List[Union[int, str]]]] = None,
+ chart_ids: Optional[Union[int, List[Union[int, str]]]] = None,
+ ):
+ super().__init__(model_ids=model_ids or [],
export_related=export_related)
+ self.dashboard_ids = dashboard_ids
+ self.chart_ids = chart_ids
+
+ def run(self) -> Iterator[tuple[str, Callable[[], str]]]:
+ if not feature_flag_manager.is_feature_enabled("TAGGING_SYSTEM"):
+ return
+
+ self.validate()
+
+ dashboard_ids: list[int] = (
+ [self.dashboard_ids]
+ if isinstance(self.dashboard_ids, int)
+ else list(self.dashboard_ids or [])
+ )
+ chart_ids: list[int] = (
+ [self.chart_ids]
+ if isinstance(self.chart_ids, int)
+ else list(self.chart_ids or [])
+ )
+
+ if self.model_ids:
+ for tag in self._models:
+ for tagged_object in tag.objects:
+ if tagged_object.object_type == ObjectType.dashboard:
+ dashboard_ids.append(tagged_object.object_id)
+ elif tagged_object.object_type == ObjectType.chart:
+ chart_ids.append(tagged_object.object_id)
+
+ dashboard_ids = list(set(dashboard_ids))
+ chart_ids = list(set(chart_ids))
Review Comment:
Fair point, swapped the set() calls for dict.fromkeys() so we still de-dupe
but keep insertion order. Pushed in e924e37.
--
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]
