mikebridge opened a new pull request, #41910: URL: https://github.com/apache/superset/pull/41910
> ⛔ **Depends on #41176 (versioning base infrastructure) — do NOT merge until that lands.** This fix touches `superset/versioning/baseline/insertion.py`, which is introduced by #41176, so this branch is **stacked on it**; the diff below includes #41176's changes until it merges, at which point this rebases to a one-line diff. Kept as a **draft** until then. ### SUMMARY Parent baseline capture in `superset/versioning/baseline/insertion.py` runs its direct-SQL inserts under `try/except` + `no_autoflush` but **not** a SAVEPOINT. On PostgreSQL a failed statement aborts the entire transaction even when the exception is caught, so a baseline-insert failure would poison the user's save — contradicting the "versioning must never break a save" guarantee this subsystem is built around. This wraps the inserts in `session.connection().begin_nested()`, so a failure rolls back to the savepoint and the outer transaction stays healthy — degrading to "no baseline row" instead of a broken save. It mirrors the guard the change-record persist path (`changes/listener.py`) and the child-baseline path (`baseline/collection.py:133`) already use; the parent-baseline path was the one spot missing it. The affected path only runs with `ENABLE_VERSIONING_CAPTURE` on (the base infra ships dark/off), so this is a **pre-flag-flip correctness fix, not a deploy-time regression** — hence a follow-up rather than a change to #41176 at its merge gate. Surfaced during a multi-model review pass on #41176. ### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF N/A — backend transaction-safety fix. ### TESTING INSTRUCTIONS Mirrors an existing, tested pattern (`baseline/collection.py`), and the versioning capture integration tests (`tests/integration_tests/versioning/`) exercise the baseline path, so CI covers the happy path. A dedicated fault-injection test — simulate a baseline insert failure and assert the outer save still commits on PostgreSQL — would strengthen it further and is a reasonable add before the capture flag is flipped on. ### ADDITIONAL INFORMATION - [ ] Has associated issue: - [x] Required feature flags: `ENABLE_VERSIONING_CAPTURE` (the affected code path only runs with capture on) - [ ] Changes UI - [ ] Includes DB Migration - [ ] Introduces new feature or API - [ ] Removes existing feature or API 🤖 Generated with [Claude Code](https://claude.com/claude-code) -- 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]
