Skip to content

Commit

Permalink
BigQuery:
Browse files Browse the repository at this point in the history
        - Send a SupersetErrorsException instead of Test one since in the future we will be raising just superset exceptions
        - Update testsq
  • Loading branch information
Antonio-RiveroMartnez committed Oct 26, 2022
1 parent 3909223 commit 5a11829
Show file tree
Hide file tree
Showing 5 changed files with 64 additions and 27 deletions.
6 changes: 5 additions & 1 deletion superset/databases/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -72,10 +72,12 @@
from superset.databases.utils import get_table_metadata
from superset.db_engine_specs import get_available_engine_specs
from superset.errors import ErrorLevel, SupersetError, SupersetErrorType
from superset.exceptions import SupersetErrorsException
from superset.extensions import security_manager
from superset.models.core import Database
from superset.superset_typing import FlaskResponse
from superset.utils.core import error_msg_from_exception, parse_js_uri_path_item
from superset.views.base import json_errors_response
from superset.views.base_api import (
BaseSupersetModelRestApi,
requires_form_data,
Expand Down Expand Up @@ -221,7 +223,7 @@ class DatabaseRestApi(BaseSupersetModelRestApi):
log_to_statsd=False,
)
@requires_json
def post(self) -> Response:
def post(self) -> FlaskResponse:
"""Creates a new Database
---
post:
Expand Down Expand Up @@ -278,6 +280,8 @@ def post(self) -> Response:
return self.response_422(message=ex.normalized_messages())
except DatabaseConnectionFailedError as ex:
return self.response_422(message=str(ex))
except SupersetErrorsException as ex:
return json_errors_response(errors=ex.errors, status=ex.status)
except DatabaseCreateFailedError as ex:
logger.error(
"Error creating model %s: %s",
Expand Down
3 changes: 2 additions & 1 deletion superset/databases/commands/create.py
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
)
from superset.databases.commands.test_connection import TestConnectionDatabaseCommand
from superset.databases.dao import DatabaseDAO
from superset.exceptions import SupersetErrorsException
from superset.extensions import db, event_logger, security_manager

logger = logging.getLogger(__name__)
Expand All @@ -46,7 +47,7 @@ def run(self) -> Model:
try:
# Test connection before starting create transaction
TestConnectionDatabaseCommand(self._properties).run()
except DatabaseConnectionFailedError as ex:
except SupersetErrorsException as ex:
event_logger.log_with_context(
action=f"db_creation_failed.{ex.__class__.__name__}",
engine=self._properties.get("sqlalchemy_uri", "").split(":")[0],
Expand Down
23 changes: 6 additions & 17 deletions superset/databases/commands/test_connection.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,16 +27,18 @@

from superset.commands.base import BaseCommand
from superset.databases.commands.exceptions import (
DatabaseConnectionFailedError,
DatabaseSecurityUnsafeError,
DatabaseTestConnectionDriverError,
DatabaseTestConnectionFailedError,
DatabaseTestConnectionUnexpectedError,
)
from superset.databases.dao import DatabaseDAO
from superset.databases.utils import make_url_safe
from superset.errors import ErrorLevel, SupersetErrorType
from superset.exceptions import SupersetSecurityException, SupersetTimeoutException
from superset.exceptions import (
SupersetErrorsException,
SupersetSecurityException,
SupersetTimeoutException,
)
from superset.extensions import event_logger
from superset.models.core import Database

Expand Down Expand Up @@ -150,20 +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)
for error in errors:
# 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 and we don't want to
# impact other places (tests for example) at least until we discuss
# and standardize the way we bubble up exceptions
if (
error.error_type
== SupersetErrorType.CONNECTION_DATABASE_PERMISSIONS_ERROR
):
# Raise original exception with the message
raise DatabaseConnectionFailedError(error.message) from ex
raise DatabaseTestConnectionFailedError(errors) from ex
raise SupersetErrorsException(errors) from ex
except SupersetSecurityException as ex:
event_logger.log_with_context(
action=f"test_connection_error.{ex.__class__.__name__}",
Expand Down
54 changes: 49 additions & 5 deletions tests/integration_tests/databases/api_tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -525,11 +525,55 @@ def test_create_database_conn_fail(self):
self.login(username="admin")
response = self.client.post(uri, json=database_data)
response_data = json.loads(response.data.decode("utf-8"))
expected_response = {
"message": "Connection failed, please check your connection settings"
superset_error_mysql = SupersetError(
message='Either the username "superset" or the password is incorrect.',
error_type="CONNECTION_ACCESS_DENIED_ERROR",
level="error",
extra={
"engine_name": "MySQL",
"invalid": ["username", "password"],
"issue_codes": [
{
"code": 1014,
"message": (
"Issue 1014 - Either the username or the password is wrong."
),
},
{
"code": 1015,
"message": (
"Issue 1015 - Issue 1015 - Either the database is spelled incorrectly or does not exist."
),
},
],
},
)
superset_error_postgres = SupersetError(
message='The password provided for username "superset" is incorrect.',
error_type="CONNECTION_INVALID_PASSWORD_ERROR",
level="error",
extra={
"engine_name": "PostgreSQL",
"invalid": ["username", "password"],
"issue_codes": [
{
"code": 1013,
"message": (
"Issue 1013 - The password provided when connecting to a database is not valid."
),
}
],
},
)
expected_response_mysql = {"errors": [dataclasses.asdict(superset_error_mysql)]}
expected_response_postgres = {
"errors": [dataclasses.asdict(superset_error_postgres)]
}
self.assertEqual(response.status_code, 422)
self.assertEqual(response_data, expected_response)
self.assertEqual(response.status_code, 500)
if example_db.backend == "mysql":
self.assertEqual(response_data, expected_response_mysql)
else:
self.assertEqual(response_data, expected_response_postgres)

def test_update_database(self):
"""
Expand Down Expand Up @@ -1516,7 +1560,7 @@ def test_test_connection_failed_invalid_hostname(
url = "api/v1/database/test_connection/"
rv = self.post_assert_metric(url, data, "test_connection")

assert rv.status_code == 422
assert rv.status_code == 500
assert rv.headers["Content-Type"] == "application/json; charset=utf-8"
response = json.loads(rv.data.decode("utf-8"))
expected_response = {"errors": [dataclasses.asdict(superset_error)]}
Expand Down
5 changes: 2 additions & 3 deletions tests/integration_tests/databases/commands_tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@
DatabaseNotFoundError,
DatabaseSecurityUnsafeError,
DatabaseTestConnectionDriverError,
DatabaseTestConnectionFailedError,
DatabaseTestConnectionUnexpectedError,
)
from superset.databases.commands.export import ExportDatabasesCommand
Expand Down Expand Up @@ -683,7 +682,7 @@ def test_connection_do_ping_exception(
json_payload = {"sqlalchemy_uri": db_uri}
command_without_db_name = TestConnectionDatabaseCommand(json_payload)

with pytest.raises(DatabaseTestConnectionFailedError) as excinfo:
with pytest.raises(SupersetErrorsException) as excinfo:
command_without_db_name.run()
assert (
excinfo.value.errors[0].error_type
Expand Down Expand Up @@ -757,7 +756,7 @@ def test_connection_db_api_exc(
json_payload = {"sqlalchemy_uri": db_uri}
command_without_db_name = TestConnectionDatabaseCommand(json_payload)

with pytest.raises(DatabaseTestConnectionFailedError) as excinfo:
with pytest.raises(SupersetErrorsException) as excinfo:
command_without_db_name.run()
assert str(excinfo.value) == (
"Connection failed, please check your connection settings"
Expand Down

0 comments on commit 5a11829

Please sign in to comment.