You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by "eschutho (via GitHub)" <gi...@apache.org> on 2023/04/29 00:14:56 UTC

[GitHub] [superset] eschutho commented on a diff in pull request #21838: feat(bigquery): Custom message when Service Account doesnt have the correct Roles and Permissions

eschutho commented on code in PR #21838:
URL: https://github.com/apache/superset/pull/21838#discussion_r1180890561


##########
superset/databases/commands/test_connection.py:
##########
@@ -145,7 +152,7 @@ def ping(engine: Engine) -> bool:
             )
             # check for custom errors (wrong username, wrong password, etc)
             errors = database.db_engine_spec.extract_errors(ex, context)
-            raise DatabaseTestConnectionFailedError(errors) from ex
+            raise SupersetErrorsException(errors) from ex

Review Comment:
   @Antonio-RiveroMartnez I'm looking into some 500 errors for test connections, and I saw this change. What would happen if I were to change this back to raise the `DatabaseTestConnectionFailedError` which is a 422? Would it break anything from this PR? I checked through the implementation and it looks like it should still catch the SupersetErrorsExceptions because it is the base class, but I wanted to see if I might be missing anything here. 



-- 
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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org