mikebridge commented on PR #40129: URL: https://github.com/apache/superset/pull/40129#issuecomment-4606701871
Thanks @richardfogaca (and the PR review agent — looping you back in via that path) — both of your suggestions are addressed: **`UPDATING.md:41` — rollback edge for soft-deleted rows**: added a rollback paragraph in `7f15310b2d`: > *"Rollback note: if the application code is rolled back after charts have been soft-deleted, the older code path's visibility filter no longer applies and previously hidden rows become visible to the older code. Pair the rollback with a data decision (restore the rows, hard-delete them, or also downgrade the migration) rather than assuming the old hard-delete semantics still hold."* Matches the shape you described — operator pairs rollback with a data plan, not assumed old semantics. **`superset/commands/chart/importers/v1/utils.py:81` — inline `find_existing_for_import` import**: moved to module top (same commit). Verified no circular dependency exists between `superset.commands.importers.v1.utils` and `superset.commands.chart.importers.v1.utils` before moving. Per constitution v1.3.2 (recently updated in this fork), inline imports require a `# avoid circular import` or `# avoid app-init regression` justification comment; without that justification, module-top is the default. Your reading was correct on the first pass. Also thanks for the praise on the dashboard-junction regression test — the two-sided contract check was deliberate and reading it back from your review made me appreciate the framing more. -- 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]
