codeant-ai-for-open-source[bot] commented on code in PR #42339:
URL: https://github.com/apache/superset/pull/42339#discussion_r3679658407
##########
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:
**Suggestion:** The nested tag export runs once per chart, but
`ExportChartsCommand.run()` de-duplicates yielded filenames across all charts.
When exporting multiple charts directly, the first chart's `tags.yaml` is
retained and tags from every later chart are discarded. Aggregate the chart IDs
and emit one tag export, or otherwise merge the tag data before filename
de-duplication. [incomplete implementation]
<details>
<summary><b>Severity Level:</b> Critical 🚨</summary>
```mdx
- ❌ Direct multi-chart exports omit later charts' tags.
- ⚠️ Exported bundles contain incomplete `tags.yaml`.
- ⚠️ Dashboard exports avoid the bug via separate aggregation.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=9581d067e3214865807a99d482536f9a&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
[](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=9581d067e3214865807a99d482536f9a&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
*(Use Cmd/Ctrl + Click for best experience)*
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset/commands/chart/export.py
**Line:** 113:113
**Comment:**
*Incomplete Implementation: The nested tag export runs once per chart,
but `ExportChartsCommand.run()` de-duplicates yielded filenames across all
charts. When exporting multiple charts directly, the first chart's `tags.yaml`
is retained and tags from every later chart are discarded. Aggregate the chart
IDs and emit one tag export, or otherwise merge the tag data before filename
de-duplication.
Validate the correctness of the flagged issue. If correct, How can I resolve
this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask
user if the user wants to fix the rest of the comments as well. if said yes,
then fetch all the comments validate the correctness and implement a minimal fix
```
</details>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F42339&comment_hash=a431d07ba752053d22b5354c968d4244c358e8f26f43f083d5562cc7be52d148&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F42339&comment_hash=a431d07ba752053d22b5354c968d4244c358e8f26f43f083d5562cc7be52d148&reaction=dislike'>👎</a>
##########
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:
**Suggestion:** Converting IDs through `set` makes their iteration order
nondeterministic. Since `_file_content()` queries dashboards and charts in this
order and preserves the first-seen tag order in the YAML payload, identical
exports can produce different `tags.yaml` ordering across runs, causing
unstable export bundles and diffs. Preserve insertion order while
de-duplicating, or sort the IDs before generating the content. [logic error]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ⚠️ All-assets exports can produce unstable `tags.yaml` ordering.
- ⚠️ Version-control diffs include order-only tag changes.
- ⚠️ Duplicate tag descriptions depend on traversal order.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=a21ffceaaf7a438bbf4524346fbd6915&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
[](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=a21ffceaaf7a438bbf4524346fbd6915&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
*(Use Cmd/Ctrl + Click for best experience)*
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset/commands/tag/export.py
**Line:** 74:75
**Comment:**
*Logic Error: Converting IDs through `set` makes their iteration order
nondeterministic. Since `_file_content()` queries dashboards and charts in this
order and preserves the first-seen tag order in the YAML payload, identical
exports can produce different `tags.yaml` ordering across runs, causing
unstable export bundles and diffs. Preserve insertion order while
de-duplicating, or sort the IDs before generating the content.
Validate the correctness of the flagged issue. If correct, How can I resolve
this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask
user if the user wants to fix the rest of the comments as well. if said yes,
then fetch all the comments validate the correctness and implement a minimal fix
```
</details>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F42339&comment_hash=980cee0370f6b410894687ebfecf5da75d2651b9b5dbdfabf545a6620019e98f&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F42339&comment_hash=980cee0370f6b410894687ebfecf5da75d2651b9b5dbdfabf545a6620019e98f&reaction=dislike'>👎</a>
##########
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:
**Suggestion:** The refactored class inherits from `ExportModelsCommand`,
whose interface only provides `run()` and does not define the legacy static
`export()` method. As a result, existing callers of
`ExportTagsCommand.export(...)` now fail with `AttributeError`, despite the
stated backward-compatibility requirement. Add an explicit compatibility
wrapper that delegates to the new command interface. [api mismatch]
<details>
<summary><b>Severity Level:</b> Critical 🚨</summary>
```mdx
- ❌ Legacy tag-export integrations fail with `AttributeError`.
- ⚠️ External export tooling cannot use the compatibility API.
- ⚠️ Existing internal callers no longer exercise the old interface.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=dfa31753fe1e4015a17588f04208cbc2&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
[](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=dfa31753fe1e4015a17588f04208cbc2&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
*(Use Cmd/Ctrl + Click for best experience)*
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset/commands/tag/export.py
**Line:** 33:33
**Comment:**
*Api Mismatch: The refactored class inherits from
`ExportModelsCommand`, whose interface only provides `run()` and does not
define the legacy static `export()` method. As a result, existing callers of
`ExportTagsCommand.export(...)` now fail with `AttributeError`, despite the
stated backward-compatibility requirement. Add an explicit compatibility
wrapper that delegates to the new command interface.
Validate the correctness of the flagged issue. If correct, How can I resolve
this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask
user if the user wants to fix the rest of the comments as well. if said yes,
then fetch all the comments validate the correctness and implement a minimal fix
```
</details>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F42339&comment_hash=f3f4f09f7b0e10d20f8c38edca7bd9e0e86044d29d73d07b7b775d99415497e2&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F42339&comment_hash=f3f4f09f7b0e10d20f8c38edca7bd9e0e86044d29d73d07b7b775d99415497e2&reaction=dislike'>👎</a>
--
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]
