EnxDev commented on code in PR #43334:
URL: https://github.com/apache/superset/pull/43334#discussion_r3852264313
##########
superset/commands/dataset/create.py:
##########
@@ -50,7 +54,25 @@ def run(self) -> Model:
self.validate()
dataset = DatasetDAO.create(attributes=self._properties)
- dataset.fetch_metadata()
+ try:
+ dataset.fetch_metadata()
+ except SupersetException as ex:
+ # Not a SQLAlchemyError, so ``on_error`` re-raises it untouched and
+ # it escapes to FAB's ``@safe`` as an opaque 500 "Fatal error".
+ # Deliberately covers the 403 ``SupersetSecurityException`` raised
+ # for mutation/multi-statement SQL too: ``validate()`` already
+ # reports that class of rejection as a 422 on ``sql`` via
+ # ``DatasetDataAccessIsNotAllowed``.
+ raise DatasetInvalidError(
Review Comment:
Confirmed true for the primary path `CreateDatasetCommand.run()` now wraps
`fetch_metadata()` failures into DatasetInvalidError before they reach that
branch.
However, I initially deleted the branch and that broke an existing test
(test_create_virtual_dataset_sql_error_is_actionable), which exercises it as a
deliberate defensive backstop for callers that raise
SupersetGenericDBErrorException directly (mocked or otherwise).
Restored the branch with a one-line comment explaining why it's kept
despite being currently unreachable via that specific path.
--
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]
