fitzee commented on PR #40008:
URL: https://github.com/apache/superset/pull/40008#issuecomment-4417700782
**Design review notes for upstream reviewers**
I did a thorough pre-merge review of this PR and wanted to surface a few
things that might save reviewers time:
**Why dual-write (`datasetId` + `datasetUuid` both in the exported YAML)**
The export intentionally preserves `datasetId` alongside the new
`datasetUuid` so that bundles exported after this change remain importable by
older Superset instances that don't yet have the matching importer fix. UUID is
authoritative on import — new importers overwrite `datasetId` from
`dataset_info`; old importers fall back to the original (env-specific) integer
ID, which is the same broken behavior as before this fix, not worse.
**Why `target.pop("datasetId", None)` in the unresolvable-UUID else branch**
When a target has `datasetUuid` present but the UUID can't be resolved from
`dataset_info` (e.g. incomplete bundle), the stale `datasetId` is also removed.
A display control with no dataset reference is visibly broken and diagnosable;
one silently bound to whatever dataset happens to own that integer ID in the
destination environment is a data-correctness hazard.
**Known follow-up: duplicate dataset exports**
If multiple display controls reference the same dataset,
`ExportDatasetsCommand([dataset_id]).run()` fires once per target. The ZIP
overwrite is harmless (same content), but the redundant DB queries and export
work are wasteful. This is pre-existing behavior in the native filter export
loop. Worth a follow-up issue but not a blocker here.
--
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]
