mikebridge commented on PR #44015:
URL: https://github.com/apache/superset/pull/44015#issuecomment-5591627186
@aminghadersohi — reworked at `3d52c50dc3` in response to the independent
coordinator pass; ready for another look when you have a moment.
The earlier `refresh(entity, with_for_update=True)` approach had two defects
the cross-model review caught:
- a `refresh()` that finds no row raises `InvalidRequestError` (a
`SQLAlchemyError`), **not** `ObjectDeletedError` — so the hard-delete catch
never fired and `on_error` wrapped it into a 422 instead of the documented 404;
- a concurrent **soft** delete was invisible to `refresh()` (SoftDeleteMixin
column loads bypass the global `deleted_at IS NULL` filter), so the revert
would resurrect the archived row and report success.
Both are folded into a single locking re-read that re-asserts the active-row
predicate:
```python
entity = (
db.session.query(self.model_cls)
.populate_existing() # reload current committed
state
.filter_by(id=entity.id, deleted_at=None) # exclude soft-deleted rows
.with_for_update() # serialise the revert (MySQL
RR)
.one_or_none()
)
if entity is None:
raise self.not_found_exc() # hard- or soft-deleted -> 404
```
One statement now closes all three races (concurrent edit / hard delete /
soft delete) with no dependency on which exception a missing-row read raises.
Tests: the unit suite asserts the full locking chain (`populate_existing` +
`filter_by(deleted_at=None)` + `with_for_update` + `one_or_none`), the
validate→read→resolve→restore order, and `None → not_found_exc` — all
dialect-independent. Two real second-connection integration tests (hard delete
and soft delete committed in the validate→lock window) each assert
`not_found_exc`, not a silent success or 422. mypy/ruff/pylint clean on changed
files. CI is in flight.
--
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]
