EnxDev commented on code in PR #43232:
URL: https://github.com/apache/superset/pull/43232#discussion_r4142905626
##########
superset/commands/chart/export.py:
##########
@@ -108,12 +158,54 @@ def run(self) -> Iterator[tuple[str, Callable[[], str]]]:
@staticmethod
def _export(
- model: Slice, export_related: bool = True
+ model: Slice,
+ export_related: bool = True,
+ _seen: set[int] | None = None,
) -> Iterator[tuple[str, Callable[[], str]]]:
+ # Guard against circular annotation references (A→B→A).
+ # _seen is passed down the call stack so no class-level state is
needed.
+ if _seen is None:
+ _seen = set()
+ if model.id in _seen:
+ return
+ _seen.add(model.id)
yield (
ExportChartsCommand._file_name(model),
lambda: ExportChartsCommand._file_content(model),
)
if model.table and export_related:
yield from ExportDatasetsCommand([model.table.id]).run()
+
+ # Parse params once for deck_multi and annotation layer handling
+ try:
+ model_params = json.loads(model.params or "{}")
+ except json.JSONDecodeError:
+ model_params = {}
+ annotation_layers = model_params.get("annotation_layers", [])
+
+ # Export charts referenced as annotation sources (table/line
sourceType)
+ if export_related and annotation_layers:
+ chart_annotation_ids = [
+ layer["value"]
+ for layer in annotation_layers
+ if layer.get("sourceType")
+ in ANNOTATION_SOURCE_TYPES_WITH_CHART_REFERENCE
+ and isinstance(layer.get("value"), int)
+ ]
Review Comment:
Fixed in 87e98f9. Dependencies are now collected from params and from
query_context.
##########
superset/commands/chart/importers/v1/utils.py:
##########
@@ -271,3 +314,147 @@ def migrate_chart(config: dict[str, Any]) -> dict[str,
Any]:
output["query_context"] = json.dumps(query_context)
return output
+
+
+def topological_sort_charts(
+ chart_configs: list[dict[str, Any]],
+) -> list[dict[str, Any]]:
+ """Sort charts so that annotation dependencies are imported first.
+
+ Handles multi-level dependencies (A→B→C) by iteratively resolving
+ charts whose in-batch dependencies are already satisfied.
+
+ TODO: Add runtime circular annotation detection in
+ QueryContextProcessor.get_viz_annotation_data to prevent infinite
+ recursion when rendering charts with circular line annotations.
+ """
+ if len(chart_configs) <= 1:
+ return chart_configs
+
+ def _annotation_dependencies(chart_config: dict[str, Any]) -> set[str]:
+ refs = {
+ ann["value"]
+ for ann in chart_config.get("params", {}).get("annotation_layers",
[])
+ if ann.get("sourceType") in
ANNOTATION_SOURCE_TYPES_WITH_CHART_REFERENCE
+ and isinstance(ann.get("value"), str)
+ }
+ if query_context_raw := chart_config.get("query_context"):
+ try:
+ query_context = json.loads(query_context_raw)
+ except (json.JSONDecodeError, TypeError):
+ query_context = {}
+
+ for query in query_context.get("queries", []):
+ refs.update(
+ ann["value"]
+ for ann in query.get("annotation_layers", [])
+ if ann.get("sourceType")
+ in ANNOTATION_SOURCE_TYPES_WITH_CHART_REFERENCE
+ and isinstance(ann.get("value"), str)
+ )
+ refs.update(
+ ann["value"]
+ for ann in query_context.get("form_data", {}).get(
+ "annotation_layers", []
+ )
+ if ann.get("sourceType") in
ANNOTATION_SOURCE_TYPES_WITH_CHART_REFERENCE
+ and isinstance(ann.get("value"), str)
+ )
+ return refs
+
+ batch_uuids = {c["uuid"] for c in chart_configs}
+ sorted_refs: list[dict[str, Any]] = []
+ remaining = list(chart_configs)
+ resolved: set[str] = set()
+ while remaining:
+ next_remaining = []
+ for c in remaining:
+ unmet = _annotation_dependencies(c).intersection(batch_uuids -
resolved)
+ if not unmet:
+ sorted_refs.append(c)
+ resolved.add(c["uuid"])
+ else:
+ next_remaining.append(c)
+ if len(next_remaining) == len(remaining):
+ logger.warning(
+ "Circular annotation dependency detected for charts: %s — "
+ "these charts may have unresolved annotation references after
import.",
+ [c["uuid"] for c in next_remaining],
+ )
+ sorted_refs.extend(next_remaining)
Review Comment:
Fixed in 87e98f9. Charts are imported in dependency order, and references to
charts that come later in the bundle are kept as UUIDs until every chart
exists; a second pass then resolves them.
`test_import_chart_circular_chart_annotation_references_keep_both_sides` covers
A and B pointing at each other in params and query_context.
##########
superset/utils/core.py:
##########
@@ -211,6 +211,18 @@ class AnnotationType(StrEnum):
TIME_SERIES = "TIME_SERIES"
+# Annotation source types whose ``value`` field references another Chart
+# (resolved to a local Slice.id on import / serialised back to UUID on export).
+# Add new chart-referencing source types here; all consumers pick them up
+# automatically via this single definition.
+ANNOTATION_SOURCE_TYPES_WITH_CHART_REFERENCE: frozenset[str] = frozenset(
+ {
+ "table",
+ "line",
+ }
+)
Review Comment:
Already handled: both spots in `query_context_processor.py` use the constant.
##########
superset/commands/annotation_layer/importers/v1/__init__.py:
##########
@@ -0,0 +1,55 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+from typing import Any, Optional
+
+from marshmallow import Schema
+from sqlalchemy.orm import Session # noqa: F401
Review Comment:
Already gone, the module doesn't import `Session` anymore.
--
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]
