betodealmeida opened a new pull request #13960: URL: https://github.com/apache/superset/pull/13960
### SUMMARY <!--- Describe the change below, including rationale and design decisions --> This PR is a first step in making the v1 API compatible with [SIP-40](https://github.com/apache/superset/issues/9194) and [SIP-41](https://github.com/apache/superset/issues/9298). Currently, the API returns exceptions using helper methods from Flask-AppBuilder, eg: ```python try: TestConnectionDatabaseCommand(g.user, item).run() return self.response(200, message="OK") except DatabaseTestConnectionFailedError as ex: return self.response_422(message=str(ex)) except SupersetErrorException as ex: return self.response(ex.status, message=ex.error.message) ``` These responses are simple JSON payloads, not compatible with the standard error format introduced in SIP-40. For example, when adding a database with a non-existent driver (`foo://`) this is returned: ```json {"message": "Could not load database driver: BaseEngineSpec"} ``` And this is the message displayed in the frontend (in a toast): > ERROR: Could not load database driver: BaseEngineSpec To conform with SIP-41 I added a new decorator called `handle_exception` that captures exceptions and formats them according to SIP-40. This allows us to simplify the API logic, removing the need to capture exceptions when running commands (see the new `test_connection` method in this PR). With the decorator, the backend now returns: ```json { "errors": [ { "message": "Could not load database driver: BaseEngineSpec", "error_type": "GENERIC_COMMAND_ERROR", "level": "error", "extra": { "issue_codes": [ { "code": 1010, "message": "Issue 1010 - Superset found an error while running a command." } ] } } ] } ``` And the message is displayed correctly in the frontend as before, in a toast, without requiring any modifications: > ERROR: Could not load database driver: BaseEngineSpec ### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF <!--- Skip this if not applicable --> See above. ### TEST PLAN <!--- What steps should be taken to verify the changes --> Will add unit tests, I wanted to discuss this first. ### ADDITIONAL INFORMATION <!--- Check any relevant boxes with "x" --> <!--- HINT: Include "Fixes #nnn" if you are fixing an existing issue --> - [X] Has associated issue: https://github.com/apache/superset/issues/11882 - [ ] 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. For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
