aminghadersohi commented on code in PR #44253:
URL: https://github.com/apache/superset/pull/44253#discussion_r4024251909
##########
superset/commands/version_restore.py:
##########
@@ -81,7 +83,20 @@ def run(self) -> RestoreResult:
# reference ``self.failed_exc`` — a per-subclass ClassVar that
# isn't available when this method is defined on the base (same
# pattern and rationale as ``BaseRestoreCommand.run``).
- @transaction(on_error=partial(on_error, reraise=self.failed_exc))
+ # ``catches`` widens past the SQLAlchemyError default so the
+ # restore engine's fail-closed registry guard (``LookupError``
+ # for a model missing from ``_RESTORE_RELATIONS``) maps to
+ # ``failed_exc`` → 422 instead of a raw 500 (sc-115326). The
+ # tuple is deliberately this narrow: other non-SQLAlchemy
+ # exceptions must keep passing through untouched for the
+ # endpoint to map explicitly.
+ @transaction(
+ on_error=partial(
+ on_error,
+ catches=(SQLAlchemyError, LookupError),
Review Comment:
bito's LookupError⊃KeyError/IndexError point is mechanically right, but I
couldn't reach one: in _do_restore() + transaction()'s commit(), dict reads are
.get()/pre-seeded, row indices fixed-arity, the change listener fail-open, and
Continuum's version_class catches its own KeyError.
--
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]
