hughhhh commented on code in PR #21838:
URL: https://github.com/apache/superset/pull/21838#discussion_r1000878190
##########
superset/databases/commands/create.py:
##########
@@ -46,6 +48,34 @@ def run(self) -> Model:
try:
# Test connection before starting create transaction
TestConnectionDatabaseCommand(self._properties).run()
+ except DatabaseTestConnectionFailedError as ex:
+ event_logger.log_with_context(
+ action=f"db_creation_failed.{ex.__class__.__name__}",
+ engine=self._properties.get("sqlalchemy_uri",
"").split(":")[0],
+ )
+ # if the exception is DatabaseTestConnectionFailedError it has
errors in it
+ # so we try to return the message from the errors instead of using
the
+ # default message
+ error_list: List[SupersetError] = ex.errors
+ for error in error_list:
+ # We could have just set the message coming from the original
exception
+ # But that would have impacted all other places that relies on
the
+ # current message. Like api_tests for postgres etc. In the
future,
+ # after we discuss whether we want to show the original message
+ # all the time we can simply remove the check and set the
message
+ # for all cases.
+ # Why Adding the message for this error type?
+ # Because it's the type returned for Service Accounts without
roles
+ # or permissions when connecting to BigQuery DBs, which is
+ # the change we are introducing at this moment.
+ if (
+ error.error_type
+ == SupersetErrorType.CONNECTION_DATABASE_PERMISSIONS_ERROR
+ ):
+ raise DatabaseConnectionFailedError(
Review Comment:
can we just raise the original `DatabaseTestConnectionFailedError`
--
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]
