rusackas commented on code in PR #40588:
URL: https://github.com/apache/superset/pull/40588#discussion_r3349963506
##########
superset/commands/dashboard/export.py:
##########
@@ -105,6 +106,120 @@ def append_charts(position: dict[str, Any], charts:
set[Slice]) -> dict[str, Any
return position
+# Bound the derived id to a signed 31-bit positive integer so it stays well
+# within the range a database auto-increment primary key would occupy and
+# never collides with the sign bit. The concrete value is irrelevant — only
+# its stability across environments matters.
+_STABLE_CHART_ID_MODULO = 2_147_483_647
+
+
+def stable_chart_id(chart_uuid: str) -> int:
+ """Derive a deterministic, environment-independent integer from a chart
UUID.
+
+ The dashboard export format historically embedded ``meta.chartId`` — the
+ source environment's auto-increment primary key — inside every ``CHART-*``
+ position node (issue #32972). That integer differs between environments, so
+ re-exporting an imported dashboard produced a different bundle for the same
+ logical content. Deriving the id from the (stable) UUID instead makes the
+ export reproducible while still giving the importer an integer it can use
to
+ rewire the legacy, integer-keyed metadata references back to local IDs.
+ """
+ return (uuid_module.UUID(chart_uuid).int % _STABLE_CHART_ID_MODULO) + 1
Review Comment:
Good catch — fixed. `_stabilize_chart_ids` now wraps the `stable_chart_id()`
call in a `try/except ValueError`: a CHART node with a malformed `meta.uuid` is
logged and skipped (keeping its original chartId) rather than aborting the
whole export. Added `test_stabilize_chart_ids_skips_invalid_uuid` to lock it in.
##########
superset/commands/dashboard/export.py:
##########
@@ -105,6 +106,120 @@ def append_charts(position: dict[str, Any], charts:
set[Slice]) -> dict[str, Any
return position
+# Bound the derived id to a signed 31-bit positive integer so it stays well
+# within the range a database auto-increment primary key would occupy and
+# never collides with the sign bit. The concrete value is irrelevant — only
+# its stability across environments matters.
+_STABLE_CHART_ID_MODULO = 2_147_483_647
+
+
+def stable_chart_id(chart_uuid: str) -> int:
+ """Derive a deterministic, environment-independent integer from a chart
UUID.
+
+ The dashboard export format historically embedded ``meta.chartId`` — the
+ source environment's auto-increment primary key — inside every ``CHART-*``
+ position node (issue #32972). That integer differs between environments, so
+ re-exporting an imported dashboard produced a different bundle for the same
+ logical content. Deriving the id from the (stable) UUID instead makes the
+ export reproducible while still giving the importer an integer it can use
to
+ rewire the legacy, integer-keyed metadata references back to local IDs.
+ """
+ return (uuid_module.UUID(chart_uuid).int % _STABLE_CHART_ID_MODULO) + 1
+
+
+def _stabilize_chart_ids(payload: dict[str, Any]) -> None:
+ """Replace env-local integer chart IDs in ``payload`` with UUID-derived
ones.
+
+ Rewrites ``meta.chartId`` in every ``CHART`` position node to a value
derived
+ from ``meta.uuid`` and remaps the legacy, integer-keyed metadata references
+ (filter scopes, default filters, expanded slices, etc.) accordingly so the
+ bundle stays internally consistent. Mappings are applied defensively — a
+ reference whose source id is unknown is dropped rather than raising — so a
+ partially corrupt position never aborts the export. See ``stable_chart_id``
+ and issue #32972 for the motivation.
+ """
+ position = payload.get("position")
+ if not isinstance(position, dict):
+ return
+
+ # Map each chart's env-local integer id -> stable UUID-derived id.
+ id_map: dict[int, int] = {}
+ for node in position.values():
+ if (
+ isinstance(node, dict)
+ and node.get("type") == "CHART"
+ and isinstance(node.get("meta"), dict)
+ ):
+ meta = node["meta"]
+ chart_uuid = meta.get("uuid")
+ old_id = meta.get("chartId")
+ if chart_uuid is None:
+ continue
+ new_id = stable_chart_id(str(chart_uuid))
+ if isinstance(old_id, int):
+ id_map[old_id] = new_id
+ meta["chartId"] = new_id
Review Comment:
Valid — fixed. `_stabilize_chart_ids` now remaps
`native_filter_configuration[*].scope.excluded` and `chartsInScope` using the
same `id_map`, so on import these align with the stabilized position IDs
instead of being dropped. Covered by
`test_stabilize_chart_ids_remaps_native_filter_scope`.
##########
superset/commands/dashboard/export.py:
##########
@@ -182,6 +297,11 @@ def _file_content(model: Dashboard) -> str:
if orphan_charts:
payload["position"] = append_charts(payload["position"],
orphan_charts)
+ # Strip env-local integer chart IDs in favor of UUID-derived ones so
the
+ # export is reproducible across environments (issue #32972). Must run
+ # after orphan charts are appended so their nodes are stabilized too.
+ _stabilize_chart_ids(payload)
Review Comment:
Valid — fixed. `_stabilize_chart_ids` now also remaps
`global_chart_configuration` (scope.excluded/chartsInScope) and re-keys
`chart_configuration` (its outer key, inner `id`, and `crossFilters` scope) via
the same `id_map`, matching what `update_cross_filter_scoping` expects on
import. Covered by `test_stabilize_chart_ids_remaps_cross_filter_configuration`.
--
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]
