rusackas commented on code in PR #42339:
URL: https://github.com/apache/superset/pull/42339#discussion_r3772491180
##########
superset/commands/tag/export.py:
##########
@@ -23,33 +23,56 @@
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.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,
+ ):
+ self.model_ids = model_ids or []
+ self.export_related = export_related
+ self.dashboard_ids = dashboard_ids
+ self.chart_ids = chart_ids
+ self._models = []
+
+ def run(self) -> Iterator[tuple[str, Callable[[], str]]]:
+ if not feature_flag_manager.is_feature_enabled("TAGGING_SYSTEM"):
+ return
+
+ yield (
+ ExportTagsCommand._file_name(),
+ lambda: ExportTagsCommand._file_content(
+ self.dashboard_ids, self.chart_ids
+ ),
+ )
Review Comment:
Fixed in 48e4fbf7d2 — run() now walks self._models (populated via the
inherited validate()/dao.find_by_ids) and folds each tagged object's
dashboard/chart id into the export.
##########
superset/commands/tag/export.py:
##########
@@ -23,33 +23,56 @@
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.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,
+ ):
+ self.model_ids = model_ids or []
+ self.export_related = export_related
+ self.dashboard_ids = dashboard_ids
+ self.chart_ids = chart_ids
Review Comment:
The find_by_ids() call happens via the inherited validate(), called at the
top of run() — fixed alongside the above in 48e4fbf7d2. The Optional[list[int]]
widening on __init__ is intentional (None defaults to [] before reaching the
parent), not a contract violation.
##########
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:
Fixed in e924e370 — ExportChartsCommand.run() now aggregates tag export
across all requested charts after super().run(), instead of once per chart from
inside _export().
##########
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:
export() had no remaining callers in-repo (verified: assets.py,
chart/export.py, dashboard/export.py all migrated to run() in this same PR), so
it was removed rather than kept as a compat shim. Corrected the PR description,
which still claimed it was preserved.
##########
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:
Fixed in e924e370 — swapped set() for dict.fromkeys(), so de-duping
preserves insertion order instead of depending on set iteration order.
--
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]
