From ae953d4ef2034f2a97a0a2d0c536c17a8c0328c9 Mon Sep 17 00:00:00 2001 From: Beto Dealmeida Date: Wed, 12 May 2021 18:32:10 -0700 Subject: [PATCH] feat: API endpoint to validate databases using separate parameters (#14420) * feat: new endpoint for validating database parameters * Rebase * Remove broken tests --- .../pages/docs/Miscellaneous/issue_codes.mdx | 24 +++ .../src/components/ErrorMessage/types.ts | 5 + superset/config.py | 2 +- superset/constants.py | 1 + superset/databases/api.py | 57 +++++++ superset/databases/commands/exceptions.py | 14 +- superset/databases/commands/validate.py | 127 +++++++++++++++ superset/databases/schemas.py | 32 +++- superset/db_engine_specs/base.py | 73 ++++++++- superset/db_engine_specs/bigquery.py | 5 +- superset/db_engine_specs/cockroachdb.py | 1 + superset/db_engine_specs/mssql.py | 8 +- superset/db_engine_specs/mysql.py | 6 +- superset/db_engine_specs/postgres.py | 7 + superset/db_engine_specs/presto.py | 10 +- superset/db_engine_specs/redshift.py | 8 +- superset/errors.py | 30 ++++ superset/exceptions.py | 27 ++++ tests/databases/api_tests.py | 150 +++++++++++++++++- tests/databases/commands_tests.py | 127 ++++++++++++++- .../db_engine_specs/base_engine_spec_tests.py | 101 ++++++++++++ tests/db_engine_specs/postgres_tests.py | 37 +++-- 22 files changed, 811 insertions(+), 41 deletions(-) create mode 100644 superset/databases/commands/validate.py diff --git a/docs/src/pages/docs/Miscellaneous/issue_codes.mdx b/docs/src/pages/docs/Miscellaneous/issue_codes.mdx index 6b4a72d10bb2..ad26c70a798e 100644 --- a/docs/src/pages/docs/Miscellaneous/issue_codes.mdx +++ b/docs/src/pages/docs/Miscellaneous/issue_codes.mdx @@ -183,3 +183,27 @@ The user doesn't have the proper permissions to connect to the database ``` We were unable to connect to your database. Please confirm that your service account has the Viewer and Job User roles on the project. + +## Issue 1018 + +``` +One or more parameters needed to configure a database are missing. +``` + +Not all parameters required to test, create, or edit a database were present. Please double check which parameters are needed, and that they are present. + +## Issue 1019 + +``` +The submitted payload has the incorrect format. +``` + +Please check that the request payload has the correct format (eg, JSON). + +## Issue 1020 + +``` +The submitted payload has the incorrect schema. +``` + +Please check that the request payload has the expected schema. diff --git a/superset-frontend/src/components/ErrorMessage/types.ts b/superset-frontend/src/components/ErrorMessage/types.ts index 19c485b0c06d..a2ce66b594b1 100644 --- a/superset-frontend/src/components/ErrorMessage/types.ts +++ b/superset-frontend/src/components/ErrorMessage/types.ts @@ -38,6 +38,7 @@ export const ErrorTypeEnum = { CONNECTION_UNKNOWN_DATABASE_ERROR: 'CONNECTION_UNKNOWN_DATABASE_ERROR', CONNECTION_DATABASE_PERMISSIONS_ERROR: 'CONNECTION_DATABASE_PERMISSIONS_ERROR', + CONNECTION_MISSING_PARAMETERS_ERRORS: 'CONNECTION_MISSING_PARAMETERS_ERRORS', // Viz errors VIZ_GET_DF_ERROR: 'VIZ_GET_DF_ERROR', @@ -60,6 +61,10 @@ export const ErrorTypeEnum = { // Generic errors GENERIC_COMMAND_ERROR: 'GENERIC_COMMAND_ERROR', GENERIC_BACKEND_ERROR: 'GENERIC_BACKEND_ERROR', + + // API errors + INVALID_PAYLOAD_FORMAT_ERROR: 'INVALID_PAYLOAD_FORMAT_ERROR', + INVALID_PAYLOAD_SCHEMA_ERROR: 'INVALID_PAYLOAD_SCHEMA_ERROR', } as const; type ValueOf = T[keyof T]; diff --git a/superset/config.py b/superset/config.py index 89caeada0fa1..9863f76838c0 100644 --- a/superset/config.py +++ b/superset/config.py @@ -630,7 +630,7 @@ def _try_json_readsha( # pylint: disable=unused-argument # Default row limit for SQL Lab queries. Is overridden by setting a new limit in # the SQL Lab UI -DEFAULT_SQLLAB_LIMIT = 1000 +DEFAULT_SQLLAB_LIMIT = 10000 # Maximum number of tables/views displayed in the dropdown window in SQL Lab. MAX_TABLE_NAMES = 3000 diff --git a/superset/constants.py b/superset/constants.py index 5fd59bd2896f..a4f0ad133b39 100644 --- a/superset/constants.py +++ b/superset/constants.py @@ -106,6 +106,7 @@ class RouteMethod: # pylint: disable=too-few-public-methods "select_star": "read", "table_metadata": "read", "test_connection": "read", + "validate_parameters": "read", "favorite_status": "read", "thumbnail": "read", "import_": "write", diff --git a/superset/databases/api.py b/superset/databases/api.py index 5b964e68c21d..370e513e25e3 100644 --- a/superset/databases/api.py +++ b/superset/databases/api.py @@ -47,6 +47,7 @@ from superset.databases.commands.importers.dispatcher import ImportDatabasesCommand from superset.databases.commands.test_connection import TestConnectionDatabaseCommand from superset.databases.commands.update import UpdateDatabaseCommand +from superset.databases.commands.validate import ValidateDatabaseParametersCommand from superset.databases.dao import DatabaseDAO from superset.databases.decorators import check_datasource_access from superset.databases.filters import DatabaseFilter @@ -57,6 +58,7 @@ DatabasePutSchema, DatabaseRelatedObjectsResponse, DatabaseTestConnectionSchema, + DatabaseValidateParametersSchema, get_export_ids_schema, SchemasResponseSchema, SelectStarResponseSchema, @@ -65,6 +67,7 @@ from superset.databases.utils import get_table_metadata from superset.db_engine_specs import get_available_engine_specs from superset.db_engine_specs.base import BaseParametersMixin +from superset.exceptions import InvalidPayloadFormatError, InvalidPayloadSchemaError from superset.extensions import security_manager from superset.models.core import Database from superset.typing import FlaskResponse @@ -87,6 +90,7 @@ class DatabaseRestApi(BaseSupersetModelRestApi): "related_objects", "function_names", "available", + "validate_parameters", } resource_name = "database" class_permission_name = "Database" @@ -176,6 +180,7 @@ class DatabaseRestApi(BaseSupersetModelRestApi): DatabaseFunctionNamesResponse, DatabaseRelatedObjectsResponse, DatabaseTestConnectionSchema, + DatabaseValidateParametersSchema, TableMetadataResponseSchema, SelectStarResponseSchema, SchemasResponseSchema, @@ -914,3 +919,55 @@ def available(self) -> Response: ) return self.response(200, databases=available_databases) + + @expose("/validate_parameters", methods=["POST"]) + @protect() + @statsd_metrics + @event_logger.log_this_with_context( + action=lambda self, *args, **kwargs: f"{self.__class__.__name__}" + f".validate_parameters", + log_to_statsd=False, + ) + def validate_parameters( # pylint: disable=too-many-return-statements + self, + ) -> FlaskResponse: + """validates database connection parameters + --- + post: + description: >- + Validates parameters used to connect to a database + requestBody: + description: DB-specific parameters + required: true + content: + application/json: + schema: + $ref: "#/components/schemas/DatabaseValidateParametersSchema" + responses: + 200: + description: Database Test Connection + content: + application/json: + schema: + type: object + properties: + message: + type: string + 400: + $ref: '#/components/responses/400' + 422: + $ref: '#/components/responses/422' + 500: + $ref: '#/components/responses/500' + """ + if not request.is_json: + raise InvalidPayloadFormatError("Request is not JSON") + + try: + payload = DatabaseValidateParametersSchema().load(request.json) + except ValidationError as error: + raise InvalidPayloadSchemaError(error) + + command = ValidateDatabaseParametersCommand(g.user, payload) + command.run() + return self.response(200, message="OK") diff --git a/superset/databases/commands/exceptions.py b/superset/databases/commands/exceptions.py index e4236cfb3c56..c7df6bc3f1b9 100644 --- a/superset/databases/commands/exceptions.py +++ b/superset/databases/commands/exceptions.py @@ -25,7 +25,7 @@ ImportFailedError, UpdateFailedError, ) -from superset.exceptions import SupersetErrorsException +from superset.exceptions import SupersetErrorException, SupersetErrorsException class DatabaseInvalidError(CommandInvalidError): @@ -137,3 +137,15 @@ class DatabaseTestConnectionUnexpectedError(SupersetErrorsException): class DatabaseImportError(ImportFailedError): message = _("Import database failed for an unknown reason") + + +class InvalidEngineError(SupersetErrorException): + status = 422 + + +class DatabaseOfflineError(SupersetErrorException): + status = 422 + + +class InvalidParametersError(SupersetErrorsException): + status = 422 diff --git a/superset/databases/commands/validate.py b/superset/databases/commands/validate.py new file mode 100644 index 000000000000..b6aa974cdca5 --- /dev/null +++ b/superset/databases/commands/validate.py @@ -0,0 +1,127 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. +from contextlib import closing +from typing import Any, Dict, Optional + +from flask_appbuilder.security.sqla.models import User +from flask_babel import gettext as __ +from sqlalchemy.engine.url import make_url + +from superset.commands.base import BaseCommand +from superset.databases.commands.exceptions import ( + DatabaseOfflineError, + DatabaseTestConnectionFailedError, + InvalidEngineError, + InvalidParametersError, +) +from superset.databases.dao import DatabaseDAO +from superset.db_engine_specs import get_engine_specs +from superset.db_engine_specs.base import BaseParametersMixin +from superset.errors import ErrorLevel, SupersetError, SupersetErrorType +from superset.models.core import Database + + +class ValidateDatabaseParametersCommand(BaseCommand): + def __init__(self, user: User, parameters: Dict[str, Any]): + self._actor = user + self._properties = parameters.copy() + self._model: Optional[Database] = None + + def run(self) -> None: + engine = self._properties["engine"] + engine_specs = get_engine_specs() + if engine not in engine_specs: + raise InvalidEngineError( + SupersetError( + message=__( + 'Engine "%(engine)s" is not a valid engine.', engine=engine, + ), + error_type=SupersetErrorType.GENERIC_DB_ENGINE_ERROR, + level=ErrorLevel.ERROR, + extra={"allowed": list(engine_specs), "provided": engine}, + ), + ) + engine_spec = engine_specs[engine] + if not issubclass(engine_spec, BaseParametersMixin): + raise InvalidEngineError( + SupersetError( + message=__( + 'Engine "%(engine)s" cannot be configured through parameters.', + engine=engine, + ), + error_type=SupersetErrorType.GENERIC_DB_ENGINE_ERROR, + level=ErrorLevel.ERROR, + extra={ + "allowed": [ + name + for name, engine_spec in engine_specs.items() + if issubclass(engine_spec, BaseParametersMixin) + ], + "provided": engine, + }, + ), + ) + + # perform initial validation + errors = engine_spec.validate_parameters(self._properties["parameters"]) + if errors: + raise InvalidParametersError(errors) + + # try to connect + sqlalchemy_uri = engine_spec.build_sqlalchemy_uri( + self._properties["parameters"] # type: ignore + ) + if self._model and sqlalchemy_uri == self._model.safe_sqlalchemy_uri(): + sqlalchemy_uri = self._model.sqlalchemy_uri_decrypted + database = DatabaseDAO.build_db_for_connection_test( + server_cert=self._properties.get("server_cert", ""), + extra=self._properties.get("extra", "{}"), + impersonate_user=self._properties.get("impersonate_user", False), + encrypted_extra=self._properties.get("encrypted_extra", "{}"), + ) + database.set_sqlalchemy_uri(sqlalchemy_uri) + database.db_engine_spec.mutate_db_for_connection_test(database) + username = self._actor.username if self._actor is not None else None + engine = database.get_sqla_engine(user_name=username) + try: + with closing(engine.raw_connection()) as conn: + alive = engine.dialect.do_ping(conn) + except Exception as ex: # pylint: disable=broad-except + url = make_url(sqlalchemy_uri) + context = { + "hostname": url.host, + "password": url.password, + "port": url.port, + "username": url.username, + "database": url.database, + } + errors = database.db_engine_spec.extract_errors(ex, context) + raise DatabaseTestConnectionFailedError(errors) + + if not alive: + raise DatabaseOfflineError( + SupersetError( + message=__("Database is offline."), + error_type=SupersetErrorType.GENERIC_DB_ENGINE_ERROR, + level=ErrorLevel.ERROR, + ), + ) + + def validate(self) -> None: + database_name = self._properties.get("database_name") + if database_name is not None: + self._model = DatabaseDAO.get_database_by_name(database_name) diff --git a/superset/databases/schemas.py b/superset/databases/schemas.py index b39e2b113a37..ffe4a8a04e15 100644 --- a/superset/databases/schemas.py +++ b/superset/databases/schemas.py @@ -213,9 +213,9 @@ class DatabaseParametersSchemaMixin: """ Allow SQLAlchemy URI to be passed as separate parameters. - This mixing is a first step in allowing the users to test, create and + This mixin is a first step in allowing the users to test, create and edit databases without having to know how to write a SQLAlchemy URI. - Instead, each databases defines the parameters that it takes (eg, + Instead, each database defines the parameters that it takes (eg, username, password, host, etc.) and the SQLAlchemy URI is built from these parameters. @@ -223,7 +223,7 @@ class DatabaseParametersSchemaMixin: """ parameters = fields.Dict( - keys=fields.Str(), + keys=fields.String(), values=fields.Raw(), description="DB-specific parameters for configuration", ) @@ -270,10 +270,34 @@ def build_sqlalchemy_uri( ] ) - data["sqlalchemy_uri"] = engine_spec.build_sqlalchemy_url(parameters) + data["sqlalchemy_uri"] = engine_spec.build_sqlalchemy_uri(parameters) return data +class DatabaseValidateParametersSchema(Schema): + engine = fields.String(required=True, description="SQLAlchemy engine to use") + parameters = fields.Dict( + keys=fields.String(), + values=fields.Raw(), + description="DB-specific parameters for configuration", + ) + database_name = fields.String( + description=database_name_description, allow_none=True, validate=Length(1, 250), + ) + impersonate_user = fields.Boolean(description=impersonate_user_description) + extra = fields.String(description=extra_description, validate=extra_validator) + encrypted_extra = fields.String( + description=encrypted_extra_description, + validate=encrypted_extra_validator, + allow_none=True, + ) + server_cert = fields.String( + description=server_cert_description, + allow_none=True, + validate=server_cert_validator, + ) + + class DatabasePostSchema(Schema, DatabaseParametersSchemaMixin): class Meta: # pylint: disable=too-few-public-methods unknown = EXCLUDE diff --git a/superset/db_engine_specs/base.py b/superset/db_engine_specs/base.py index 7473029dd607..c60e6d70fbe1 100644 --- a/superset/db_engine_specs/base.py +++ b/superset/db_engine_specs/base.py @@ -63,6 +63,7 @@ from superset.utils import core as utils from superset.utils.core import ColumnSpec, GenericDataType from superset.utils.hashing import md5_sha_from_str +from superset.utils.network import is_hostname_valid, is_port_open if TYPE_CHECKING: # prevent circular imports @@ -269,7 +270,9 @@ class BaseEngineSpec: # pylint: disable=too-many-public-methods max_column_name_length = 0 try_remove_schema_from_table_name = True # pylint: disable=invalid-name run_multiple_statements_as_one = False - custom_errors: Dict[Pattern[str], Tuple[str, SupersetErrorType]] = {} + custom_errors: Dict[ + Pattern[str], Tuple[str, SupersetErrorType, Dict[str, Any]] + ] = {} @classmethod def get_dbapi_exception_mapping(cls) -> Dict[Type[Exception], Type[Exception]]: @@ -727,16 +730,17 @@ def extract_errors( raw_message = cls._extract_error_message(ex) context = context or {} - for regex, (message, error_type) in cls.custom_errors.items(): + for regex, (message, error_type, extra) in cls.custom_errors.items(): match = regex.search(raw_message) if match: params = {**context, **match.groupdict()} + extra["engine_name"] = cls.engine_name return [ SupersetError( error_type=error_type, message=message % params, level=ErrorLevel.ERROR, - extra={"engine_name": cls.engine_name}, + extra=extra, ) ] @@ -1274,13 +1278,13 @@ def get_column_spec( # schema for adding a database by providing parameters instead of the # full SQLAlchemy URI class BaseParametersSchema(Schema): - username = fields.String(allow_none=True, description=__("Username")) + username = fields.String(required=True, allow_none=True, description=__("Username")) password = fields.String(allow_none=True, description=__("Password")) host = fields.String(required=True, description=__("Hostname or IP address")) port = fields.Integer(required=True, description=__("Database port")) database = fields.String(required=True, description=__("Database name")) query = fields.Dict( - keys=fields.Str(), values=fields.Raw(), description=__("Additinal parameters") + keys=fields.Str(), values=fields.Raw(), description=__("Additional parameters") ) @@ -1318,7 +1322,7 @@ class BaseParametersMixin: ) @classmethod - def build_sqlalchemy_url(cls, parameters: BaseParametersType) -> str: + def build_sqlalchemy_uri(cls, parameters: BaseParametersType) -> str: return str( URL( cls.drivername, @@ -1331,8 +1335,8 @@ def build_sqlalchemy_url(cls, parameters: BaseParametersType) -> str: ) ) - @classmethod - def get_parameters_from_uri(cls, uri: str) -> BaseParametersType: + @staticmethod + def get_parameters_from_uri(uri: str) -> BaseParametersType: url = make_url(uri) return { "username": url.username, @@ -1343,6 +1347,59 @@ def get_parameters_from_uri(cls, uri: str) -> BaseParametersType: "query": url.query, } + @classmethod + def validate_parameters(cls, parameters: BaseParametersType) -> List[SupersetError]: + """ + Validates any number of parameters, for progressive validation. + + If only the hostname is present it will check if the name is resolvable. As more + parameters are present in the request, more validation is done. + """ + errors: List[SupersetError] = [] + + required = {"host", "port", "username", "database"} + present = {key for key in parameters if parameters[key]} # type: ignore + missing = sorted(required - present) + + if missing: + errors.append( + SupersetError( + message=f'One or more parameters are missing: {", ".join(missing)}', + error_type=SupersetErrorType.CONNECTION_MISSING_PARAMETERS_ERROR, + level=ErrorLevel.WARNING, + extra={"missing": missing}, + ), + ) + + host = parameters["host"] + if not host: + return errors + if not is_hostname_valid(host): + errors.append( + SupersetError( + message="The hostname provided can't be resolved.", + error_type=SupersetErrorType.CONNECTION_INVALID_HOSTNAME_ERROR, + level=ErrorLevel.ERROR, + extra={"invalid": ["host"]}, + ), + ) + return errors + + port = parameters["port"] + if not port: + return errors + if not is_port_open(host, port): + errors.append( + SupersetError( + message="The port is closed.", + error_type=SupersetErrorType.CONNECTION_PORT_CLOSED_ERROR, + level=ErrorLevel.ERROR, + extra={"invalid": ["port"]}, + ), + ) + + return errors + @classmethod def parameters_json_schema(cls) -> Any: """ diff --git a/superset/db_engine_specs/bigquery.py b/superset/db_engine_specs/bigquery.py index 7b8fa84e6bcb..9c52b44eca7b 100644 --- a/superset/db_engine_specs/bigquery.py +++ b/superset/db_engine_specs/bigquery.py @@ -16,7 +16,7 @@ # under the License. import re from datetime import datetime -from typing import Any, Dict, List, Optional, Tuple, TYPE_CHECKING +from typing import Any, Dict, List, Optional, Pattern, Tuple, TYPE_CHECKING import pandas as pd from flask_babel import gettext as __ @@ -95,7 +95,7 @@ class BigQueryEngineSpec(BaseEngineSpec): "P1Y": "{func}({col}, YEAR)", } - custom_errors = { + custom_errors: Dict[Pattern[str], Tuple[str, SupersetErrorType, Dict[str, Any]]] = { CONNECTION_DATABASE_PERMISSIONS_REGEX: ( __( "We were unable to connect to your database. Please " @@ -103,6 +103,7 @@ class BigQueryEngineSpec(BaseEngineSpec): "and Job User roles on the project." ), SupersetErrorType.CONNECTION_DATABASE_PERMISSIONS_ERROR, + {}, ), } diff --git a/superset/db_engine_specs/cockroachdb.py b/superset/db_engine_specs/cockroachdb.py index f2f00c1a0478..80b547bd8d1a 100644 --- a/superset/db_engine_specs/cockroachdb.py +++ b/superset/db_engine_specs/cockroachdb.py @@ -20,3 +20,4 @@ class CockroachDbEngineSpec(PostgresEngineSpec): engine = "cockroachdb" engine_name = "CockroachDB" + drivername = "cockroach" diff --git a/superset/db_engine_specs/mssql.py b/superset/db_engine_specs/mssql.py index 9e089d435078..c24d40d23b51 100644 --- a/superset/db_engine_specs/mssql.py +++ b/superset/db_engine_specs/mssql.py @@ -17,7 +17,7 @@ import logging import re from datetime import datetime -from typing import Any, List, Optional, Tuple +from typing import Any, Dict, List, Optional, Pattern, Tuple from flask_babel import gettext as __ @@ -64,21 +64,24 @@ class MssqlEngineSpec(BaseEngineSpec): "P1Y": "DATEADD(year, DATEDIFF(year, 0, {col}), 0)", } - custom_errors = { + custom_errors: Dict[Pattern[str], Tuple[str, SupersetErrorType, Dict[str, Any]]] = { CONNECTION_ACCESS_DENIED_REGEX: ( __( 'Either the username "%(username)s", password, ' 'or database name "%(database)s" is incorrect.' ), SupersetErrorType.CONNECTION_ACCESS_DENIED_ERROR, + {}, ), CONNECTION_INVALID_HOSTNAME_REGEX: ( __('The hostname "%(hostname)s" cannot be resolved.'), SupersetErrorType.CONNECTION_INVALID_HOSTNAME_ERROR, + {}, ), CONNECTION_PORT_CLOSED_REGEX: ( __('Port %(port)s on hostname "%(hostname)s" refused the connection.'), SupersetErrorType.CONNECTION_PORT_CLOSED_ERROR, + {}, ), CONNECTION_HOST_DOWN_REGEX: ( __( @@ -86,6 +89,7 @@ class MssqlEngineSpec(BaseEngineSpec): "reached on port %(port)s." ), SupersetErrorType.CONNECTION_HOST_DOWN_ERROR, + {}, ), } diff --git a/superset/db_engine_specs/mysql.py b/superset/db_engine_specs/mysql.py index a5d4a16052dd..d5e5a5c75610 100644 --- a/superset/db_engine_specs/mysql.py +++ b/superset/db_engine_specs/mysql.py @@ -107,22 +107,26 @@ class MySQLEngineSpec(BaseEngineSpec): type_code_map: Dict[int, str] = {} # loaded from get_datatype only if needed - custom_errors = { + custom_errors: Dict[Pattern[str], Tuple[str, SupersetErrorType, Dict[str, Any]]] = { CONNECTION_ACCESS_DENIED_REGEX: ( __('Either the username "%(username)s" or the password is incorrect.'), SupersetErrorType.CONNECTION_ACCESS_DENIED_ERROR, + {}, ), CONNECTION_INVALID_HOSTNAME_REGEX: ( __('Unknown MySQL server host "%(hostname)s".'), SupersetErrorType.CONNECTION_INVALID_HOSTNAME_ERROR, + {}, ), CONNECTION_HOST_DOWN_REGEX: ( __('The host "%(hostname)s" might be down and can\'t be reached.'), SupersetErrorType.CONNECTION_HOST_DOWN_ERROR, + {}, ), CONNECTION_UNKNOWN_DATABASE_REGEX: ( __('Unable to connect to database "%(database)s".'), SupersetErrorType.CONNECTION_UNKNOWN_DATABASE_ERROR, + {}, ), } diff --git a/superset/db_engine_specs/postgres.py b/superset/db_engine_specs/postgres.py index abca36d5c04f..8caba10b729e 100644 --- a/superset/db_engine_specs/postgres.py +++ b/superset/db_engine_specs/postgres.py @@ -104,22 +104,27 @@ class PostgresBaseEngineSpec(BaseEngineSpec): CONNECTION_INVALID_USERNAME_REGEX: ( __('The username "%(username)s" does not exist.'), SupersetErrorType.CONNECTION_INVALID_USERNAME_ERROR, + {"invalid": ["username"]}, ), CONNECTION_INVALID_PASSWORD_REGEX: ( __('The password provided for username "%(username)s" is incorrect.'), SupersetErrorType.CONNECTION_INVALID_PASSWORD_ERROR, + {"invalid": ["username", "password"]}, ), CONNECTION_INVALID_PASSWORD_NEEDED_REGEX: ( __("Please re-enter the password."), SupersetErrorType.CONNECTION_ACCESS_DENIED_ERROR, + {"invalid": ["password"]}, ), CONNECTION_INVALID_HOSTNAME_REGEX: ( __('The hostname "%(hostname)s" cannot be resolved.'), SupersetErrorType.CONNECTION_INVALID_HOSTNAME_ERROR, + {"invalid": ["host"]}, ), CONNECTION_PORT_CLOSED_REGEX: ( __('Port %(port)s on hostname "%(hostname)s" refused the connection.'), SupersetErrorType.CONNECTION_PORT_CLOSED_ERROR, + {"invalid": ["host", "port"]}, ), CONNECTION_HOST_DOWN_REGEX: ( __( @@ -127,10 +132,12 @@ class PostgresBaseEngineSpec(BaseEngineSpec): "reached on port %(port)s." ), SupersetErrorType.CONNECTION_HOST_DOWN_ERROR, + {"invalid": ["host", "port"]}, ), CONNECTION_UNKNOWN_DATABASE_REGEX: ( __('Unable to connect to database "%(database)s".'), SupersetErrorType.CONNECTION_UNKNOWN_DATABASE_ERROR, + {"invalid": ["database"]}, ), } diff --git a/superset/db_engine_specs/presto.py b/superset/db_engine_specs/presto.py index 32741b150818..3ffe45942205 100644 --- a/superset/db_engine_specs/presto.py +++ b/superset/db_engine_specs/presto.py @@ -165,13 +165,14 @@ class PrestoEngineSpec(BaseEngineSpec): # pylint: disable=too-many-public-metho "date_add('day', 1, CAST({col} AS TIMESTAMP))))", } - custom_errors = { + custom_errors: Dict[Pattern[str], Tuple[str, SupersetErrorType, Dict[str, Any]]] = { COLUMN_DOES_NOT_EXIST_REGEX: ( __( 'We can\'t seem to resolve the column "%(column_name)s" at ' "line %(location)s.", ), SupersetErrorType.COLUMN_DOES_NOT_EXIST_ERROR, + {}, ), TABLE_DOES_NOT_EXIST_REGEX: ( __( @@ -179,6 +180,7 @@ class PrestoEngineSpec(BaseEngineSpec): # pylint: disable=too-many-public-metho "A valid table must be used to run this query.", ), SupersetErrorType.TABLE_DOES_NOT_EXIST_ERROR, + {}, ), SCHEMA_DOES_NOT_EXIST_REGEX: ( __( @@ -186,14 +188,17 @@ class PrestoEngineSpec(BaseEngineSpec): # pylint: disable=too-many-public-metho "A valid schema must be used to run this query.", ), SupersetErrorType.SCHEMA_DOES_NOT_EXIST_ERROR, + {}, ), CONNECTION_ACCESS_DENIED_REGEX: ( __('Either the username "%(username)s" or the password is incorrect.'), SupersetErrorType.CONNECTION_ACCESS_DENIED_ERROR, + {}, ), CONNECTION_INVALID_HOSTNAME_REGEX: ( __('The hostname "%(hostname)s" cannot be resolved.'), SupersetErrorType.CONNECTION_INVALID_HOSTNAME_ERROR, + {}, ), CONNECTION_HOST_DOWN_REGEX: ( __( @@ -201,14 +206,17 @@ class PrestoEngineSpec(BaseEngineSpec): # pylint: disable=too-many-public-metho "reached on port %(port)s." ), SupersetErrorType.CONNECTION_HOST_DOWN_ERROR, + {}, ), CONNECTION_PORT_CLOSED_REGEX: ( __('Port %(port)s on hostname "%(hostname)s" refused the connection.'), SupersetErrorType.CONNECTION_PORT_CLOSED_ERROR, + {}, ), CONNECTION_UNKNOWN_DATABASE_ERROR: ( __('Unable to connect to catalog named "%(catalog_name)s".'), SupersetErrorType.CONNECTION_UNKNOWN_DATABASE_ERROR, + {}, ), } diff --git a/superset/db_engine_specs/redshift.py b/superset/db_engine_specs/redshift.py index 60953f575d44..f2d2652089ee 100644 --- a/superset/db_engine_specs/redshift.py +++ b/superset/db_engine_specs/redshift.py @@ -15,6 +15,7 @@ # specific language governing permissions and limitations # under the License. import re +from typing import Any, Dict, Pattern, Tuple from flask_babel import gettext as __ @@ -49,18 +50,21 @@ class RedshiftEngineSpec(PostgresBaseEngineSpec): engine_name = "Amazon Redshift" max_column_name_length = 127 - custom_errors = { + custom_errors: Dict[Pattern[str], Tuple[str, SupersetErrorType, Dict[str, Any]]] = { CONNECTION_ACCESS_DENIED_REGEX: ( __('Either the username "%(username)s" or the password is incorrect.'), SupersetErrorType.CONNECTION_ACCESS_DENIED_ERROR, + {}, ), CONNECTION_INVALID_HOSTNAME_REGEX: ( __('The hostname "%(hostname)s" cannot be resolved.'), SupersetErrorType.CONNECTION_INVALID_HOSTNAME_ERROR, + {}, ), CONNECTION_PORT_CLOSED_REGEX: ( __('Port %(port)s on hostname "%(hostname)s" refused the connection.'), SupersetErrorType.CONNECTION_PORT_CLOSED_ERROR, + {}, ), CONNECTION_HOST_DOWN_REGEX: ( __( @@ -68,6 +72,7 @@ class RedshiftEngineSpec(PostgresBaseEngineSpec): "reached on port %(port)s." ), SupersetErrorType.CONNECTION_HOST_DOWN_ERROR, + {}, ), CONNECTION_UNKNOWN_DATABASE_REGEX: ( __( @@ -75,6 +80,7 @@ class RedshiftEngineSpec(PostgresBaseEngineSpec): " Please verify your database name and try again." ), SupersetErrorType.CONNECTION_UNKNOWN_DATABASE_ERROR, + {}, ), } diff --git a/superset/errors.py b/superset/errors.py index be05944cafaf..961cca444c98 100644 --- a/superset/errors.py +++ b/superset/errors.py @@ -48,6 +48,7 @@ class SupersetErrorType(str, Enum): CONNECTION_ACCESS_DENIED_ERROR = "CONNECTION_ACCESS_DENIED_ERROR" CONNECTION_UNKNOWN_DATABASE_ERROR = "CONNECTION_UNKNOWN_DATABASE_ERROR" CONNECTION_DATABASE_PERMISSIONS_ERROR = "CONNECTION_DATABASE_PERMISSIONS_ERROR" + CONNECTION_MISSING_PARAMETERS_ERROR = "CONNECTION_MISSING_PARAMETERS_ERROR" # Viz errors VIZ_GET_DF_ERROR = "VIZ_GET_DF_ERROR" @@ -70,6 +71,10 @@ class SupersetErrorType(str, Enum): GENERIC_COMMAND_ERROR = "GENERIC_COMMAND_ERROR" GENERIC_BACKEND_ERROR = "GENERIC_BACKEND_ERROR" + # API errors + INVALID_PAYLOAD_FORMAT_ERROR = "INVALID_PAYLOAD_FORMAT_ERROR" + INVALID_PAYLOAD_SCHEMA_ERROR = "INVALID_PAYLOAD_SCHEMA_ERROR" + ERROR_TYPES_TO_ISSUE_CODES_MAPPING = { SupersetErrorType.BACKEND_TIMEOUT_ERROR: [ @@ -220,6 +225,31 @@ class SupersetErrorType(str, Enum): "message": _("Issue 1017 - User doesn't have the proper permissions."), }, ], + SupersetErrorType.CONNECTION_MISSING_PARAMETERS_ERROR: [ + { + "code": 1018, + "message": _( + "Issue 1018 - One or more parameters needed to configure a " + "database are missing." + ), + }, + ], + SupersetErrorType.INVALID_PAYLOAD_FORMAT_ERROR: [ + { + "code": 1019, + "message": _( + "Issue 1019 - The submitted payload has the incorrect format." + ), + } + ], + SupersetErrorType.INVALID_PAYLOAD_SCHEMA_ERROR: [ + { + "code": 1020, + "message": _( + "Issue 1020 - The submitted payload has the incorrect schema." + ), + } + ], } diff --git a/superset/exceptions.py b/superset/exceptions.py index aff21a64deaf..2926817062ef 100644 --- a/superset/exceptions.py +++ b/superset/exceptions.py @@ -17,6 +17,7 @@ from typing import Any, Dict, List, Optional from flask_babel import gettext as _ +from marshmallow import ValidationError from superset.errors import ErrorLevel, SupersetError, SupersetErrorType @@ -155,3 +156,29 @@ class DashboardImportException(SupersetException): class SerializationError(SupersetException): pass + + +class InvalidPayloadFormatError(SupersetErrorException): + status = 400 + + def __init__(self, message: str = "Request payload has incorrect format"): + error = SupersetError( + message=message, + error_type=SupersetErrorType.INVALID_PAYLOAD_FORMAT_ERROR, + level=ErrorLevel.ERROR, + extra={}, + ) + super().__init__(error) + + +class InvalidPayloadSchemaError(SupersetErrorException): + status = 422 + + def __init__(self, error: ValidationError): + error = SupersetError( + message="An error happened when validating the request", + error_type=SupersetErrorType.INVALID_PAYLOAD_SCHEMA_ERROR, + level=ErrorLevel.ERROR, + extra={"messages": error.messages}, + ) + super().__init__(error) diff --git a/tests/databases/api_tests.py b/tests/databases/api_tests.py index 1ce922f09b00..d14f790f9a03 100644 --- a/tests/databases/api_tests.py +++ b/tests/databases/api_tests.py @@ -1290,7 +1290,7 @@ def test_available(self, app, get_available_engine_specs): }, "query": { "additionalProperties": {}, - "description": "Additinal parameters", + "description": "Additional parameters", "type": "object", }, "username": { @@ -1299,7 +1299,7 @@ def test_available(self, app, get_available_engine_specs): "type": "string", }, }, - "required": ["database", "host", "port"], + "required": ["database", "host", "port", "username"], "type": "object", }, "preferred": True, @@ -1308,3 +1308,149 @@ def test_available(self, app, get_available_engine_specs): {"engine": "mysql", "name": "MySQL", "preferred": False}, ] } + + def test_validate_parameters_invalid_payload_format(self): + self.login(username="admin") + url = "api/v1/database/validate_parameters" + rv = self.client.post(url, data="INVALID", content_type="text/plain") + response = json.loads(rv.data.decode("utf-8")) + + assert rv.status_code == 400 + assert response == { + "errors": [ + { + "message": "Request is not JSON", + "error_type": "INVALID_PAYLOAD_FORMAT_ERROR", + "level": "error", + "extra": { + "issue_codes": [ + { + "code": 1019, + "message": "Issue 1019 - The submitted payload has the incorrect format.", + } + ] + }, + } + ] + } + + def test_validate_parameters_invalid_payload_schema(self): + self.login(username="admin") + url = "api/v1/database/validate_parameters" + payload = {"foo": "bar"} + rv = self.client.post(url, json=payload) + response = json.loads(rv.data.decode("utf-8")) + + assert rv.status_code == 422 + assert response == { + "errors": [ + { + "message": "An error happened when validating the request", + "error_type": "INVALID_PAYLOAD_SCHEMA_ERROR", + "level": "error", + "extra": { + "messages": { + "engine": ["Missing data for required field."], + "foo": ["Unknown field."], + }, + "issue_codes": [ + { + "code": 1020, + "message": "Issue 1020 - The submitted payload has the incorrect schema.", + } + ], + }, + } + ] + } + + def test_validate_parameters_missing_fields(self): + self.login(username="admin") + url = "api/v1/database/validate_parameters" + payload = { + "engine": "postgresql", + "parameters": { + "host": "", + "port": 5432, + "username": "", + "password": "", + "database": "", + "query": {}, + }, + } + rv = self.client.post(url, json=payload) + response = json.loads(rv.data.decode("utf-8")) + + assert rv.status_code == 422 + assert response == { + "errors": [ + { + "message": "One or more parameters are missing: database, host, username", + "error_type": "CONNECTION_MISSING_PARAMETERS_ERROR", + "level": "warning", + "extra": { + "missing": ["database", "host", "username"], + "issue_codes": [ + { + "code": 1018, + "message": "Issue 1018 - One or more parameters needed to configure a database are missing.", + } + ], + }, + } + ] + } + + @mock.patch("superset.db_engine_specs.base.is_hostname_valid") + def test_validate_parameters_invalid_host(self, is_hostname_valid): + is_hostname_valid.return_value = False + + self.login(username="admin") + url = "api/v1/database/validate_parameters" + payload = { + "engine": "postgresql", + "parameters": { + "host": "localhost", + "port": 5432, + "username": "", + "password": "", + "database": "", + "query": {}, + }, + } + rv = self.client.post(url, json=payload) + response = json.loads(rv.data.decode("utf-8")) + + assert rv.status_code == 422 + assert response == { + "errors": [ + { + "message": "One or more parameters are missing: database, username", + "error_type": "CONNECTION_MISSING_PARAMETERS_ERROR", + "level": "warning", + "extra": { + "missing": ["database", "username"], + "issue_codes": [ + { + "code": 1018, + "message": "Issue 1018 - One or more parameters needed to configure a database are missing.", + } + ], + }, + }, + { + "message": "The hostname provided can't be resolved.", + "error_type": "CONNECTION_INVALID_HOSTNAME_ERROR", + "level": "error", + "extra": { + "invalid": ["host"], + "issue_codes": [ + { + "code": 1007, + "message": "Issue 1007 - The hostname provided can't be resolved.", + } + ], + }, + }, + ] + } diff --git a/tests/databases/commands_tests.py b/tests/databases/commands_tests.py index da475d11ab15..e7a3806d1de1 100644 --- a/tests/databases/commands_tests.py +++ b/tests/databases/commands_tests.py @@ -15,7 +15,7 @@ # specific language governing permissions and limitations # under the License. # pylint: disable=no-self-use, invalid-name -from unittest import mock +from unittest import mock, skip from unittest.mock import patch import pytest @@ -36,9 +36,10 @@ from superset.databases.commands.export import ExportDatabasesCommand from superset.databases.commands.importers.v1 import ImportDatabasesCommand from superset.databases.commands.test_connection import TestConnectionDatabaseCommand +from superset.databases.commands.validate import ValidateDatabaseParametersCommand from superset.databases.schemas import DatabaseTestConnectionSchema -from superset.errors import SupersetError, SupersetErrorType -from superset.exceptions import SupersetSecurityException +from superset.errors import ErrorLevel, SupersetError, SupersetErrorType +from superset.exceptions import SupersetErrorsException, SupersetSecurityException from superset.models.core import Database from superset.utils.core import backend, get_example_database from tests.base_tests import SupersetTestCase @@ -53,6 +54,7 @@ class TestExportDatabasesCommand(SupersetTestCase): + @skip("Flaky") @patch("superset.security.manager.g") @pytest.mark.usefixtures( "load_birth_names_dashboard_with_slices", "load_energy_table_with_slice" @@ -620,3 +622,122 @@ def test_connection_db_api_exc(self, mock_event_logger, mock_get_sqla_engine): ) mock_event_logger.assert_called() + + +@mock.patch("superset.db_engine_specs.base.is_hostname_valid") +@mock.patch("superset.db_engine_specs.base.is_port_open") +@mock.patch("superset.databases.commands.validate.DatabaseDAO") +def test_validate(DatabaseDAO, is_port_open, is_hostname_valid, app_context): + """ + Test parameter validation. + """ + is_hostname_valid.return_value = True + is_port_open.return_value = True + + payload = { + "engine": "postgresql", + "parameters": { + "host": "localhost", + "port": 5432, + "username": "superset", + "password": "superset", + "database": "test", + "query": {}, + }, + } + command = ValidateDatabaseParametersCommand(None, payload) + command.run() + + +@mock.patch("superset.db_engine_specs.base.is_hostname_valid") +@mock.patch("superset.db_engine_specs.base.is_port_open") +def test_validate_partial(is_port_open, is_hostname_valid, app_context): + """ + Test parameter validation when only some parameters are present. + """ + is_hostname_valid.return_value = True + is_port_open.return_value = True + + payload = { + "engine": "postgresql", + "parameters": { + "host": "localhost", + "port": 5432, + "username": "", + "password": "superset", + "database": "test", + "query": {}, + }, + } + command = ValidateDatabaseParametersCommand(None, payload) + with pytest.raises(SupersetErrorsException) as excinfo: + command.run() + assert excinfo.value.errors == [ + SupersetError( + message="One or more parameters are missing: username", + error_type=SupersetErrorType.CONNECTION_MISSING_PARAMETERS_ERROR, + level=ErrorLevel.WARNING, + extra={ + "missing": ["username"], + "issue_codes": [ + { + "code": 1018, + "message": "Issue 1018 - One or more parameters needed to configure a database are missing.", + } + ], + }, + ) + ] + + +@mock.patch("superset.db_engine_specs.base.is_hostname_valid") +def test_validate_partial_invalid_hostname(is_hostname_valid, app_context): + """ + Test parameter validation when only some parameters are present. + """ + is_hostname_valid.return_value = False + + payload = { + "engine": "postgresql", + "parameters": { + "host": "localhost", + "port": None, + "username": "", + "password": "", + "database": "", + "query": {}, + }, + } + command = ValidateDatabaseParametersCommand(None, payload) + with pytest.raises(SupersetErrorsException) as excinfo: + command.run() + assert excinfo.value.errors == [ + SupersetError( + message="One or more parameters are missing: database, port, username", + error_type=SupersetErrorType.CONNECTION_MISSING_PARAMETERS_ERROR, + level=ErrorLevel.WARNING, + extra={ + "missing": ["database", "port", "username"], + "issue_codes": [ + { + "code": 1018, + "message": "Issue 1018 - One or more parameters needed to configure a database are missing.", + } + ], + }, + ), + SupersetError( + message="The hostname provided can't be resolved.", + error_type=SupersetErrorType.CONNECTION_INVALID_HOSTNAME_ERROR, + level=ErrorLevel.ERROR, + extra={ + "invalid": ["host"], + "issue_codes": [ + { + "code": 1007, + "message": "Issue 1007 - The hostname provided can't be resolved.", + } + ], + }, + ), + ] diff --git a/tests/db_engine_specs/base_engine_spec_tests.py b/tests/db_engine_specs/base_engine_spec_tests.py index e1ca7d388dd9..164b3bbd37a0 100644 --- a/tests/db_engine_specs/base_engine_spec_tests.py +++ b/tests/db_engine_specs/base_engine_spec_tests.py @@ -22,11 +22,13 @@ from superset.db_engine_specs import get_engine_specs from superset.db_engine_specs.base import ( BaseEngineSpec, + BaseParametersMixin, builtin_time_grains, LimitMethod, ) from superset.db_engine_specs.mysql import MySQLEngineSpec from superset.db_engine_specs.sqlite import SqliteEngineSpec +from superset.errors import ErrorLevel, SupersetError, SupersetErrorType from superset.sql_parse import ParsedQuery from superset.utils.core import get_example_database from tests.db_engine_specs.base_tests import TestDbEngineSpec @@ -365,3 +367,102 @@ def test_get_time_grain_with_unkown_values(): assert list(time_grains)[-1] == "weird" app.config = config + + +@mock.patch("superset.db_engine_specs.base.is_hostname_valid") +@mock.patch("superset.db_engine_specs.base.is_port_open") +def test_validate(is_port_open, is_hostname_valid): + is_hostname_valid.return_value = True + is_port_open.return_value = True + + parameters = { + "host": "localhost", + "port": 5432, + "username": "username", + "password": "password", + "database": "dbname", + "query": {"sslmode": "verify-full"}, + } + errors = BaseParametersMixin.validate_parameters(parameters) + assert errors == [] + + +def test_validate_parameters_missing(): + parameters = { + "host": "", + "port": None, + "username": "", + "password": "", + "database": "", + "query": {}, + } + errors = BaseParametersMixin.validate_parameters(parameters) + assert errors == [ + SupersetError( + message=( + "One or more parameters are missing: " "database, host, port, username" + ), + error_type=SupersetErrorType.CONNECTION_MISSING_PARAMETERS_ERROR, + level=ErrorLevel.WARNING, + extra={"missing": ["database", "host", "port", "username"]}, + ), + ] + + +@mock.patch("superset.db_engine_specs.base.is_hostname_valid") +def test_validate_parameters_invalid_host(is_hostname_valid): + is_hostname_valid.return_value = False + + parameters = { + "host": "localhost", + "port": None, + "username": "username", + "password": "password", + "database": "dbname", + "query": {"sslmode": "verify-full"}, + } + errors = BaseParametersMixin.validate_parameters(parameters) + assert errors == [ + SupersetError( + message="One or more parameters are missing: port", + error_type=SupersetErrorType.CONNECTION_MISSING_PARAMETERS_ERROR, + level=ErrorLevel.WARNING, + extra={"missing": ["port"]}, + ), + SupersetError( + message="The hostname provided can't be resolved.", + error_type=SupersetErrorType.CONNECTION_INVALID_HOSTNAME_ERROR, + level=ErrorLevel.ERROR, + extra={"invalid": ["host"]}, + ), + ] + + +@mock.patch("superset.db_engine_specs.base.is_hostname_valid") +@mock.patch("superset.db_engine_specs.base.is_port_open") +def test_validate_parameters_port_closed(is_port_open, is_hostname_valid): + is_hostname_valid.return_value = True + is_port_open.return_value = False + + parameters = { + "host": "localhost", + "port": 5432, + "username": "username", + "password": "password", + "database": "dbname", + "query": {"sslmode": "verify-full"}, + } + errors = BaseParametersMixin.validate_parameters(parameters) + assert errors == [ + SupersetError( + message="The port is closed.", + error_type=SupersetErrorType.CONNECTION_PORT_CLOSED_ERROR, + level=ErrorLevel.ERROR, + extra={ + "invalid": ["port"], + "issue_codes": [ + {"code": 1008, "message": "Issue 1008 - The port is closed."} + ], + }, + ) + ] diff --git a/tests/db_engine_specs/postgres_tests.py b/tests/db_engine_specs/postgres_tests.py index 1dc044eee21f..11d09f155cb6 100644 --- a/tests/db_engine_specs/postgres_tests.py +++ b/tests/db_engine_specs/postgres_tests.py @@ -23,6 +23,7 @@ from superset.db_engine_specs import get_engine_specs from superset.db_engine_specs.postgres import PostgresEngineSpec from superset.errors import ErrorLevel, SupersetError, SupersetErrorType +from superset.utils.core import get_example_database from tests.db_engine_specs.base_tests import TestDbEngineSpec from tests.fixtures.certificates import ssl_certificate from tests.fixtures.database import default_db_extra @@ -234,6 +235,7 @@ def test_extract_errors(self): ), }, ], + "invalid": ["username"], }, ) ] @@ -257,6 +259,7 @@ def test_extract_errors(self): "can't be resolved.", } ], + "invalid": ["host"], }, ) ] @@ -282,6 +285,7 @@ def test_extract_errors(self): "issue_codes": [ {"code": 1008, "message": "Issue 1008 - The port is closed."} ], + "invalid": ["host", "port"], }, ) ] @@ -311,6 +315,7 @@ def test_extract_errors(self): "and can't be reached on the provided port.", } ], + "invalid": ["host", "port"], }, ) ] @@ -341,6 +346,7 @@ def test_extract_errors(self): "and can't be reached on the provided port.", } ], + "invalid": ["host", "port"], }, ) ] @@ -363,6 +369,7 @@ def test_extract_errors(self): ), }, ], + "invalid": ["username", "password"], }, ) ] @@ -385,6 +392,7 @@ def test_extract_errors(self): ), } ], + "invalid": ["database"], }, ) ] @@ -393,21 +401,20 @@ def test_extract_errors(self): result = PostgresEngineSpec.extract_errors(Exception(msg)) assert result == [ SupersetError( - error_type=SupersetErrorType.CONNECTION_ACCESS_DENIED_ERROR, message="Please re-enter the password.", + error_type=SupersetErrorType.CONNECTION_ACCESS_DENIED_ERROR, level=ErrorLevel.ERROR, extra={ + "invalid": ["password"], "engine_name": "PostgreSQL", "issue_codes": [ { "code": 1014, - "message": "Issue 1014 - Either the" - " username or the password is wrong.", + "message": "Issue 1014 - Either the username or the password is wrong.", }, { "code": 1015, - "message": "Issue 1015 - Either the database is " - "spelled incorrectly or does not exist.", + "message": "Issue 1015 - Either the database is spelled incorrectly or does not exist.", }, ], }, @@ -424,7 +431,7 @@ def test_base_parameters_mixin(): "database": "dbname", "query": {"foo": "bar"}, } - sqlalchemy_uri = PostgresEngineSpec.build_sqlalchemy_url(parameters) + sqlalchemy_uri = PostgresEngineSpec.build_sqlalchemy_uri(parameters) assert ( sqlalchemy_uri == "postgresql+psycopg2://username:password@localhost:5432/dbname?foo=bar" @@ -437,20 +444,20 @@ def test_base_parameters_mixin(): assert json_schema == { "type": "object", "properties": { + "port": { + "type": "integer", + "format": "int32", + "description": "Database port", + }, + "password": {"type": "string", "nullable": True, "description": "Password"}, "host": {"type": "string", "description": "Hostname or IP address"}, "username": {"type": "string", "nullable": True, "description": "Username"}, - "password": {"type": "string", "nullable": True, "description": "Password"}, - "database": {"type": "string", "description": "Database name"}, "query": { "type": "object", - "description": "Additinal parameters", + "description": "Additional parameters", "additionalProperties": {}, }, - "port": { - "type": "integer", - "format": "int32", - "description": "Database port", - }, + "database": {"type": "string", "description": "Database name"}, }, - "required": ["database", "host", "port"], + "required": ["database", "host", "port", "username"], }