mikebridge commented on PR #40130: URL: https://github.com/apache/superset/pull/40130#issuecomment-4897775732
Thanks all for the earlier reviews — this PR's head was refreshed and is green and ready for a final look: - **Rebased onto current master** after charts soft-delete (#40129) merged, and re-pointed the `add_deleted_at_to_tables` migration onto the new head so the branch presents a **single alembic head** (no dangling migration head). - **Addressed the outstanding automated-review comments**: the MCP `create_dataset` / `create_virtual_dataset` tools now handle the soft-deleted-twin case explicitly (actionable restore-or-rename message rather than a generic error); tightened the database-delete tests to assert the specific `DatabaseDeleteSoftDeletedDatasetsExistFailedError` (+ a live-dataset branch test); and documented that the DB `UniqueConstraint` is the real guard on the duplicate-check path. All review threads are resolved. - **Fixed a test-cleanup bug** (`_restore_dataset` was called on the wrong class in one `finally`) that had been failing the DB suites; `test-postgres` / `test-mysql` / `test-sqlite` are now green. No behavior/scope change — same soft-delete + restore, gated behind `SOFT_DELETE`. @eschutho (or any migrations code owner) — would appreciate a re-approve on the current head when you have a moment. 🙏 _This comment was generated by Claude (AI) on behalf of @mikebridge._ -- 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]
