mikebridge opened a new pull request, #45551:
URL: https://github.com/apache/superset/pull/45551
### SUMMARY
Restoring a dashboard version can fail when, at the same moment, one of the
charts it is putting back or removing gets permanently deleted (for example by
the archive's retention cleanup or a manual permanent delete). Nothing is
half-restored, since the whole restore rolls back, but the user only saw a
generic "Dashboard could not be updated." error, with no hint that simply
trying again works.
**Why it happens.** The restore first works out which charts to link and
unlink, then writes the change in one go. If one of those charts is permanently
deleted in between, the write fails: linking it violates the `dashboard_slices`
foreign key, and unlinking it finds that the row is already gone.
**What this changes.** When the restore's write to `dashboard_slices` fails
in one of those two ways, and a fresh read after the rollback confirms that one
of the charts is gone, the restore endpoint returns **409** with a message
asking the user to try again. This follows the existing convention for
retryable conflicts ("retry the same request"). The retry restores the version
without the deleted chart. Any other failure keeps the existing 422, because
retrying would not help. Successful restores are unchanged.
Also included: the 409 response in the OpenAPI spec, the new message in
`messages.pot`, and a sentence in the version history docs.
### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
N/A (API behaviour only).
### TESTING INSTRUCTIONS
- `pytest tests/integration_tests/dashboards/version_restore_tests.py -k
concurrent_chart_purge`. It runs a real permanent delete on a separate
connection between the restore's read and its write, once for a chart being
linked and once for one being unlinked. It checks for a 409, that nothing was
restored, and that a retry restores the version. A third case checks that an
unrelated foreign key failure still returns 422.
- `pytest tests/unit_tests/versioning/test_db_errors.py -k foreign_key` and
`pytest tests/unit_tests/versioning/test_restore.py -k chart_purge_race`
### ADDITIONAL INFORMATION
- [ ] Has associated issue:
- [ ] Required feature flags:
- [ ] Changes UI
- [ ] Includes DB Migration (follow approval process in
[SIP-59](https://github.com/apache/superset/issues/13351))
- [ ] Migration is atomic, supports rollback & is backwards-compatible
- [ ] Confirm DB migration upgrade and downgrade tested
- [ ] Runtime estimates and downtime expectations provided
- [ ] Introduces new feature or API
- [ ] Removes existing feature or API
--
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]
