diff --git a/docs/installation.rst b/docs/installation.rst index d4875715e..c4d3ec36a 100644 --- a/docs/installation.rst +++ b/docs/installation.rst @@ -529,7 +529,11 @@ The role and warehouse can be omitted if defaults are defined for the user, i.e. Make sure the user has privileges to access and use all required databases/schemas/tables/views/warehouses, as the Snowflake SQLAlchemy engine does -not test for user rights during engine creation. +not test for user/role rights during engine creation by default. However, when +pressing the "Test Connection" button in the Create or Edit Database dialog, +user/role credentials are validated by passing `"validate_default_parameters": True` +to the `connect()` method during engine creation. If the user/role is not authorized +to access the database, an error is recorded in the Superset logs. See `Snowflake SQLAlchemy `_. diff --git a/superset/db_engine_specs/base.py b/superset/db_engine_specs/base.py index 3e5da5d25..708dae089 100644 --- a/superset/db_engine_specs/base.py +++ b/superset/db_engine_specs/base.py @@ -942,3 +942,14 @@ class BaseEngineSpec: # pylint: disable=too-many-public-methods if data and type(data[0]).__name__ == "Row": data = [tuple(row) for row in data] return data + + @staticmethod + def mutate_db_for_connection_test(database: "Database") -> None: + """ + Some databases require passing additional parameters for validating database + connections. This method makes it possible to mutate the database instance prior + to testing if a connection is ok. + + :param database: instance to be mutated + """ + return None diff --git a/superset/db_engine_specs/snowflake.py b/superset/db_engine_specs/snowflake.py index f42267c26..090744301 100644 --- a/superset/db_engine_specs/snowflake.py +++ b/superset/db_engine_specs/snowflake.py @@ -14,14 +14,18 @@ # KIND, either express or implied. See the License for the # specific language governing permissions and limitations # under the License. +import json from datetime import datetime -from typing import Optional +from typing import Optional, TYPE_CHECKING from urllib import parse from sqlalchemy.engine.url import URL from superset.db_engine_specs.postgres import PostgresBaseEngineSpec +if TYPE_CHECKING: + from superset.models.core import Database # pylint: disable=unused-import + class SnowflakeEngineSpec(PostgresBaseEngineSpec): engine = "snowflake" @@ -77,3 +81,19 @@ class SnowflakeEngineSpec(PostgresBaseEngineSpec): if tt == "TIMESTAMP": return f"""TO_TIMESTAMP('{dttm.isoformat(timespec="microseconds")}')""" return None + + @staticmethod + def mutate_db_for_connection_test(database: "Database") -> None: + """ + By default, snowflake doesn't validate if the user/role has access to the chosen + database. + + :param database: instance to be mutated + """ + extra = json.loads(database.extra or "{}") + engine_params = extra.get("engine_params", {}) + connect_args = engine_params.get("connect_args", {}) + connect_args["validate_default_parameters"] = True + engine_params["connect_args"] = connect_args + extra["engine_params"] = engine_params + database.extra = json.dumps(extra) diff --git a/superset/views/core.py b/superset/views/core.py index ce49374a0..d5ef4cee6 100755 --- a/superset/views/core.py +++ b/superset/views/core.py @@ -1367,6 +1367,7 @@ class Superset(BaseSupersetView): encrypted_extra=json.dumps(request.json.get("encrypted_extra", {})), ) database.set_sqlalchemy_uri(uri) + database.db_engine_spec.mutate_db_for_connection_test(database) username = g.user.username if g.user is not None else None engine = database.get_sqla_engine(user_name=username) @@ -1402,7 +1403,9 @@ class Superset(BaseSupersetView): return json_error_response(_(str(e)), 400) except Exception as e: logger.error("Unexpected error %s", e) - return json_error_response(_("Unexpected error occurred."), 400) + return json_error_response( + _("Unexpected error occurred, please check your logs for details"), 400 + ) @api @has_access_api diff --git a/tests/db_engine_specs/snowflake_tests.py b/tests/db_engine_specs/snowflake_tests.py index 52d38c24c..027e37394 100644 --- a/tests/db_engine_specs/snowflake_tests.py +++ b/tests/db_engine_specs/snowflake_tests.py @@ -14,9 +14,10 @@ # KIND, either express or implied. See the License for the # specific language governing permissions and limitations # under the License. -from sqlalchemy import column +import json from superset.db_engine_specs.snowflake import SnowflakeEngineSpec +from superset.models.core import Database from tests.db_engine_specs.base_tests import DbEngineSpecTestCase @@ -24,16 +25,21 @@ class SnowflakeTestCase(DbEngineSpecTestCase): def test_convert_dttm(self): dttm = self.get_dttm() - self.assertEqual( - SnowflakeEngineSpec.convert_dttm("DATE", dttm), "TO_DATE('2019-01-02')" - ) + test_cases = { + "DATE": "TO_DATE('2019-01-02')", + "DATETIME": "CAST('2019-01-02T03:04:05.678900' AS DATETIME)", + "TIMESTAMP": "TO_TIMESTAMP('2019-01-02T03:04:05.678900')", + } - self.assertEqual( - SnowflakeEngineSpec.convert_dttm("DATETIME", dttm), - "CAST('2019-01-02T03:04:05.678900' AS DATETIME)", - ) + for type_, expected in test_cases.items(): + self.assertEqual(SnowflakeEngineSpec.convert_dttm(type_, dttm), expected) - self.assertEqual( - SnowflakeEngineSpec.convert_dttm("TIMESTAMP", dttm), - "TO_TIMESTAMP('2019-01-02T03:04:05.678900')", + def test_database_connection_test_mutator(self): + database = Database(sqlalchemy_uri="snowflake://abc") + SnowflakeEngineSpec.mutate_db_for_connection_test(database) + engine_params = json.loads(database.extra or "{}") + + self.assertDictEqual( + {"engine_params": {"connect_args": {"validate_default_parameters": True}}}, + engine_params, )