rusackas commented on PR #34880: URL: https://github.com/apache/superset/pull/34880#issuecomment-4726412066
Recapping where this landed after all the back-and-forth, for anyone coming to it fresh: The original bug (#34879) was that re-importing a dashboard hardcoded `overwrite=False` for the charts/datasets/databases inside it, so they'd never update. This fixes that, but gated behind an explicit opt-in so nothing gets clobbered by accident: - Sub-assets only overwrite when *both* `overwrite` and a new `overwrite_all` flag are set. - The import modal grows an "also overwrite all assets" checkbox, shown for dashboard imports when an overwrite is needed. - The importer's `run()` now wraps things in a commit/rollback so a failed import doesn't leave partial writes. One thing I'd want more eyes on before merging: on the API, `overwrite_all` defaults to the value of `overwrite` (so `overwrite=true` alone cascades to assets), but the CLI and command default it to `False`. So the same intent behaves differently by entrypoint, and the API default is a behavior change for existing clients... which feels like it could bite someone with shared datasets. I'm tempted to just make it opt-in (default `False`) everywhere and document the flag. Thoughts? CC @sadpandajoe (this is the "acknowledge before overwriting" thing you raised) and @betodealmeida, since you know the import/export + CLI side better than anyone. -- 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]
