From cf18d9262f12a33ebc5e021427feb0a3ac83613b Mon Sep 17 00:00:00 2001 From: Guen Prawiroatmodjo Date: Wed, 3 Apr 2024 09:57:52 -0700 Subject: [PATCH 01/19] Add database and motherduck_token fields --- superset/db_engine_specs/duckdb.py | 86 +++++++++++++++++++++++++++++- 1 file changed, 85 insertions(+), 1 deletion(-) diff --git a/superset/db_engine_specs/duckdb.py b/superset/db_engine_specs/duckdb.py index fc8efdaa31616..0b311ed800d14 100644 --- a/superset/db_engine_specs/duckdb.py +++ b/superset/db_engine_specs/duckdb.py @@ -19,16 +19,21 @@ import re from datetime import datetime from re import Pattern -from typing import Any, TYPE_CHECKING +from typing import Any, TypedDict, TYPE_CHECKING +from apispec import APISpec +from apispec.ext.marshmallow import MarshmallowPlugin from flask_babel import gettext as __ +from marshmallow import fields, Schema from sqlalchemy import types from sqlalchemy.engine.reflection import Inspector +from sqlalchemy.engine.url import URL from superset.config import VERSION_STRING from superset.constants import TimeGrain, USER_AGENT from superset.db_engine_specs.base import BaseEngineSpec from superset.errors import SupersetErrorType +from superset.errors import SupersetError if TYPE_CHECKING: # prevent circular imports @@ -102,10 +107,89 @@ def get_extra_params(database: Database) -> dict[str, Any]: return extra +# schema for adding a database by providing parameters instead of the +# full SQLAlchemy URI +class BasicParametersSchema(Schema): + database = fields.String( + required=True, metadata={"description": __("Database name")} + ) + password = fields.String(allow_none=True, metadata={"description": __("MotherDuck token")}) + + +class MotherDuckParametersType(TypedDict, total=False): + database: str + password: str + + +class MotherDuckPropertiesType(TypedDict): + parameters: MotherDuckParametersType + + class MotherDuckEngineSpec(DuckDBEngineSpec): engine = "duckdb" engine_name = "MotherDuck" + # recommended driver name for the DB engine spec + default_driver = "duckdb_engine" + sqlalchemy_uri_placeholder = ( "duckdb:///md:{database_name}?motherduck_token={SERVICE_TOKEN}" ) + + # schema describing the parameters used to configure the DB + parameters_schema = BasicParametersSchema() + + + @classmethod + def build_sqlalchemy_uri( # pylint: disable=unused-argument + cls, + parameters: MotherDuckParametersType, + encrypted_extra: dict[str, str] | None = None, + ) -> str: + return f"{cls.engine}:///md:{parameters['database']}"\ + f"?motherduck_token={parameters['password']}", + + @classmethod + def get_parameters_from_uri( # pylint: disable=unused-argument + cls, uri: str, encrypted_extra: dict[str, Any] | None = None + ) -> MotherDuckParametersType: + # pattern = "md:(?P\w+).*?motherduck_token=(?P\w+)" + # m = re.search(pattern, uri) + # return { + # "database": m.group('database'), + # "motherduck_token": m.group('token') + # } + return { + "database": "foo", + "password": "bar" + } + + @classmethod + def parameters_json_schema(cls) -> Any: + """ + Return configuration parameters as OpenAPI. + """ + if not cls.parameters_schema: + return None + + spec = APISpec( + title="Database Parameters", + version="1.0.0", + openapi_version="3.0.2", + plugins=[MarshmallowPlugin()], + ) + spec.components.schema(cls.__name__, schema=cls.parameters_schema) + return spec.to_dict()["components"]["schemas"][cls.__name__] + + @classmethod + def validate_parameters( + cls, properties: MotherDuckPropertiesType + ) -> 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] = [] + return errors From f6f0d06349449541ff41cda1450f266e9a8e241e Mon Sep 17 00:00:00 2001 From: Guen Prawiroatmodjo Date: Wed, 3 Apr 2024 17:16:51 -0700 Subject: [PATCH 02/19] add a button to get token --- .../src/components/Form/LabeledErrorBoundInput.tsx | 13 ++++++++++++- .../DatabaseConnectionForm/CommonParameters.tsx | 2 +- .../databases/DatabaseModal/ModalHeader.tsx | 2 +- 3 files changed, 14 insertions(+), 3 deletions(-) diff --git a/superset-frontend/src/components/Form/LabeledErrorBoundInput.tsx b/superset-frontend/src/components/Form/LabeledErrorBoundInput.tsx index 6b0feb7268f7e..5c80120058db9 100644 --- a/superset-frontend/src/components/Form/LabeledErrorBoundInput.tsx +++ b/superset-frontend/src/components/Form/LabeledErrorBoundInput.tsx @@ -17,7 +17,7 @@ * under the License. */ import React from 'react'; -import { Input, Tooltip } from 'antd'; +import { Input, Tooltip, Button } from 'antd'; import { styled, css, SupersetTheme, t } from '@superset-ui/core'; import InfoTooltip from 'src/components/InfoTooltip'; import Icons from 'src/components/Icons'; @@ -149,6 +149,17 @@ const LabeledErrorBoundInput = ({ ) : ( )} + {props.name === 'access_token' ? ( + + ) : ( +
+ )} ); diff --git a/superset-frontend/src/features/databases/DatabaseModal/DatabaseConnectionForm/CommonParameters.tsx b/superset-frontend/src/features/databases/DatabaseModal/DatabaseConnectionForm/CommonParameters.tsx index 3f1f5f9625fef..2c95dc9e768e2 100644 --- a/superset-frontend/src/features/databases/DatabaseModal/DatabaseConnectionForm/CommonParameters.tsx +++ b/superset-frontend/src/features/databases/DatabaseModal/DatabaseConnectionForm/CommonParameters.tsx @@ -172,7 +172,7 @@ export const accessTokenField = ({ value={db?.parameters?.access_token} validationMethods={{ onBlur: getValidation }} errorMessage={validationErrors?.access_token} - placeholder={t('e.g. ********')} + placeholder={t('Paste your access token here')} label={t('Access token')} onChange={changeMethods.onParametersChange} /> diff --git a/superset-frontend/src/features/databases/DatabaseModal/ModalHeader.tsx b/superset-frontend/src/features/databases/DatabaseModal/ModalHeader.tsx index 9825489a7b80a..d6daa2ca00330 100644 --- a/superset-frontend/src/features/databases/DatabaseModal/ModalHeader.tsx +++ b/superset-frontend/src/features/databases/DatabaseModal/ModalHeader.tsx @@ -149,7 +149,7 @@ const ModalHeader = ({ target="_blank" rel="noopener noreferrer" > - {t('connecting to %(dbModelName)s.', { dbModelName: dbModel.name })} + {t('connecting to %(dbModelName)s', { dbModelName: dbModel.name })} .

From 7a00c515ffc05d038d3d4203c24fdafba7310c10 Mon Sep 17 00:00:00 2001 From: Guen Prawiroatmodjo Date: Wed, 3 Apr 2024 17:17:25 -0700 Subject: [PATCH 03/19] add MDURI class, use instead of URL class --- superset/databases/utils.py | 5 + superset/db_engine_specs/duckdb.py | 352 ++++++++++++++++++++++------- 2 files changed, 276 insertions(+), 81 deletions(-) diff --git a/superset/databases/utils.py b/superset/databases/utils.py index 21abd7b9c283d..85d333c8d2d3a 100644 --- a/superset/databases/utils.py +++ b/superset/databases/utils.py @@ -117,6 +117,11 @@ def make_url_safe(raw_url: Union[str, URL]) -> URL: url = raw_url.strip() try: return make_url(url) # noqa + except ValueError: + if "duckdb" in url: + from superset.db_engine_specs.duckdb import MDURI + return MDURI.from_str(url) + raise DatabaseInvalidError() except Exception: raise DatabaseInvalidError() # pylint: disable=raise-missing-from diff --git a/superset/db_engine_specs/duckdb.py b/superset/db_engine_specs/duckdb.py index 0b311ed800d14..3b5158ae4b0a9 100644 --- a/superset/db_engine_specs/duckdb.py +++ b/superset/db_engine_specs/duckdb.py @@ -30,11 +30,23 @@ from sqlalchemy.engine.url import URL from superset.config import VERSION_STRING -from superset.constants import TimeGrain, USER_AGENT +from superset.constants import TimeGrain, USER_AGENT, PASSWORD_MASK from superset.db_engine_specs.base import BaseEngineSpec from superset.errors import SupersetErrorType from superset.errors import SupersetError +from apispec import APISpec +from apispec.ext.marshmallow import MarshmallowPlugin +from flask_babel import gettext as __, lazy_gettext as _ +from marshmallow import fields, Schema +from marshmallow.validate import Range +from sqlalchemy.engine.reflection import Inspector +from sqlalchemy.engine.url import URL +from sqlalchemy import util +from superset.databases.utils import make_url_safe +from superset.errors import ErrorLevel, SupersetError, SupersetErrorType +from superset.utils.network import is_hostname_valid, is_port_open + if TYPE_CHECKING: # prevent circular imports from superset.models.core import Database @@ -43,9 +55,266 @@ COLUMN_DOES_NOT_EXIST_REGEX = re.compile("no such column: (?P.+)") -class DuckDBEngineSpec(BaseEngineSpec): +class MDURI(util.namedtuple( + "MDURI", + [ + "password", + "database", + "motherduck_token", + "query", + ], + )): + drivername = "duckdb" + + def __str__(self): + query = "" + if self.motherduck_token: + query += f"?motherduck_token={self.motherduck_token}" + else: + query += "?" + query += "&".join([f"{key}={value}" for key, value in self.query.items()]) + return f"{DuckDBEngineSpec.engine}:///{self.database}{query}" + + + @classmethod + def from_str(cls, name: str): + pattern = re.compile( + r""" + (?P[\w\+]+):/// + (?: + (md:(?P[^/\?]*))? + )? + (?:\?(?P.*))? + """, + re.X, + ) + m = pattern.match(name) + if m is not None: + components = m.groupdict() + if components["query"] is not None: + query = {} + + for key, value in util.parse_qsl(components["query"]): + if util.py2k: + key = key.encode("ascii") + if key in query: + query[key] = util.to_list(query[key]) + query[key].append(value) + else: + query[key] = value + else: + query = None + if query is not None: + token = query.pop("motherduck_token") + else: + token = None + + components["query"] = query + + return cls( + password=token, + database=components.get("database"), + motherduck_token=token, + query=query + ) + + def strip(self): + return self + + def set( + self, + password=None, + database=None, + query=None, + ): + """return a new :class:`_engine.URL` object with modifications. + + Values are used if they are non-None. To set a value to ``None`` + explicitly, use the :meth:`MDURI._replace` method adapted + from ``namedtuple``. + + :param password: new password + :param database: new database + :param query: new query parameters, passed a dict of string keys + referring to string or sequence of string values. Fully + replaces the previous list of arguments. + + :return: new :class:`_engine.URL` object. + + .. versionadded:: 1.4 + + .. seealso:: + + :meth:`_engine.URL.update_query_dict` + + """ + + kw = {} + if password is not None: + kw["password"] = password + if database is not None: + kw["database"] = database + if query is not None: + kw["query"] = query + + return self._replace(**kw) + + def get_backend_name(self): + """Return the backend name. + + This is the name that corresponds to the database backend in + use, and is the portion of the :attr:`_engine.URL.drivername` + that is to the left of the plus sign. + + """ + return self.drivername + + def get_driver_name(self): + """Return the backend name. + + This is the name that corresponds to the DBAPI driver in + use, and is the portion of the :attr:`_engine.URL.drivername` + that is to the right of the plus sign. + + If the :attr:`_engine.URL.drivername` does not include a plus sign, + then the default :class:`_engine.Dialect` for this :class:`_engine.URL` + is imported in order to get the driver name. + + """ + return self.drivername + +# schema for adding a database by providing parameters instead of the +# full SQLAlchemy URI +class DuckDBParametersSchema(Schema): + access_token = fields.String(allow_none=True, metadata={"description": __("MotherDuck token")}) + database = fields.String( + required=True, metadata={"description": __("Database name")} + ) + query = fields.Dict( + keys=fields.Str(), + values=fields.Raw(), + metadata={"description": __("Additional parameters")}, + ) + + +class DuckDBParametersType(TypedDict, total=False): + access_token: str | None + database: str + query: dict[str, Any] + + +class DuckDBPropertiesType(TypedDict): + parameters: DuckDBParametersType + + +class DuckDBParametersMixin: + """ + Mixin for configuring DB engine specs via a dictionary. + + With this mixin the SQLAlchemy engine can be configured through + individual parameters, instead of the full SQLAlchemy URI. This + mixin is for DuckDB: + + duckdb:///file_path[?key=value&key=value...] + duckdb:///md:database[?key=value&key=value...] + + """ + + # schema describing the parameters used to configure the DB + parameters_schema = DuckDBParametersSchema() + + # recommended driver name for the DB engine spec + default_driver = "" + + # query parameter to enable encryption in the database connection + # for Postgres this would be `{"sslmode": "verify-ca"}`, eg. + encryption_parameters: dict[str, str] = {} + + @classmethod + def build_sqlalchemy_uri( # pylint: disable=unused-argument + cls, + parameters: DuckDBParametersType, + encrypted_extra: dict[str, str] | None = None, + ) -> str: + # make a copy so that we don't update the original + query = parameters.get("query", {}).copy() + token = parameters.get("access_token", "") + database = parameters.get("database", "") + + if database or token: + database = "md:" + database + + return MDURI(token, database, token, query) + + @classmethod + def get_parameters_from_uri( # pylint: disable=unused-argument + cls, uri: str, encrypted_extra: dict[str, Any] | None = None + ) -> DuckDBParametersType: + url = make_url_safe(uri) + query = { + key: value + for (key, value) in url.query.items() + if (key, value) not in cls.encryption_parameters.items() + } + encryption = all( + item in url.query.items() for item in cls.encryption_parameters.items() + ) + return { + "access_token": url.access_token, + "database": url.database, + "query": query, + } + + @classmethod + def validate_parameters( + cls, properties: DuckDBPropertiesType + ) -> 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 = {"access_token"} + parameters = properties.get("parameters", {}) + present = {key for key in parameters if parameters.get(key, ())} + + if missing := sorted(required - present): + 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}, + ), + ) + + return errors + + @classmethod + def parameters_json_schema(cls) -> Any: + """ + Return configuration parameters as OpenAPI. + """ + if not cls.parameters_schema: + return None + + spec = APISpec( + title="Database Parameters", + version="1.0.0", + openapi_version="3.0.2", + plugins=[MarshmallowPlugin()], + ) + spec.components.schema(cls.__name__, schema=cls.parameters_schema) + return spec.to_dict()["components"]["schemas"][cls.__name__] + + +class DuckDBEngineSpec(DuckDBParametersMixin, BaseEngineSpec): engine = "duckdb" engine_name = "DuckDB" + default_driver = "duckdb_engine" sqlalchemy_uri_placeholder = "duckdb:////path/to/duck.db" @@ -107,89 +376,10 @@ def get_extra_params(database: Database) -> dict[str, Any]: return extra -# schema for adding a database by providing parameters instead of the -# full SQLAlchemy URI -class BasicParametersSchema(Schema): - database = fields.String( - required=True, metadata={"description": __("Database name")} - ) - password = fields.String(allow_none=True, metadata={"description": __("MotherDuck token")}) - - -class MotherDuckParametersType(TypedDict, total=False): - database: str - password: str - - -class MotherDuckPropertiesType(TypedDict): - parameters: MotherDuckParametersType - - class MotherDuckEngineSpec(DuckDBEngineSpec): engine = "duckdb" engine_name = "MotherDuck" - # recommended driver name for the DB engine spec - default_driver = "duckdb_engine" - sqlalchemy_uri_placeholder = ( "duckdb:///md:{database_name}?motherduck_token={SERVICE_TOKEN}" ) - - # schema describing the parameters used to configure the DB - parameters_schema = BasicParametersSchema() - - - @classmethod - def build_sqlalchemy_uri( # pylint: disable=unused-argument - cls, - parameters: MotherDuckParametersType, - encrypted_extra: dict[str, str] | None = None, - ) -> str: - return f"{cls.engine}:///md:{parameters['database']}"\ - f"?motherduck_token={parameters['password']}", - - @classmethod - def get_parameters_from_uri( # pylint: disable=unused-argument - cls, uri: str, encrypted_extra: dict[str, Any] | None = None - ) -> MotherDuckParametersType: - # pattern = "md:(?P\w+).*?motherduck_token=(?P\w+)" - # m = re.search(pattern, uri) - # return { - # "database": m.group('database'), - # "motherduck_token": m.group('token') - # } - return { - "database": "foo", - "password": "bar" - } - - @classmethod - def parameters_json_schema(cls) -> Any: - """ - Return configuration parameters as OpenAPI. - """ - if not cls.parameters_schema: - return None - - spec = APISpec( - title="Database Parameters", - version="1.0.0", - openapi_version="3.0.2", - plugins=[MarshmallowPlugin()], - ) - spec.components.schema(cls.__name__, schema=cls.parameters_schema) - return spec.to_dict()["components"]["schemas"][cls.__name__] - - @classmethod - def validate_parameters( - cls, properties: MotherDuckPropertiesType - ) -> 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] = [] - return errors From a752454a9a3966202240268801dc51cb68893269 Mon Sep 17 00:00:00 2001 From: Guen Prawiroatmodjo Date: Thu, 4 Apr 2024 11:35:53 -0700 Subject: [PATCH 04/19] sqlalchemy uri should be str, database not required, make_url_safe returns any --- superset/databases/utils.py | 2 +- superset/db_engine_specs/duckdb.py | 7 ++++--- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/superset/databases/utils.py b/superset/databases/utils.py index 85d333c8d2d3a..5d37a5f402654 100644 --- a/superset/databases/utils.py +++ b/superset/databases/utils.py @@ -104,7 +104,7 @@ def get_table_metadata( } -def make_url_safe(raw_url: Union[str, URL]) -> URL: +def make_url_safe(raw_url: Union[str, URL]) -> Any: """ Wrapper for SQLAlchemy's make_url(), which tends to raise too detailed of errors, which inevitably find their way into server logs. ArgumentErrors diff --git a/superset/db_engine_specs/duckdb.py b/superset/db_engine_specs/duckdb.py index 3b5158ae4b0a9..7fbc6bcee6751 100644 --- a/superset/db_engine_specs/duckdb.py +++ b/superset/db_engine_specs/duckdb.py @@ -186,9 +186,10 @@ def get_driver_name(self): # schema for adding a database by providing parameters instead of the # full SQLAlchemy URI class DuckDBParametersSchema(Schema): - access_token = fields.String(allow_none=True, metadata={"description": __("MotherDuck token")}) + access_token = fields.String(allow_none=True, metadata={"description": __("MotherDuck token")} + ) database = fields.String( - required=True, metadata={"description": __("Database name")} + required=False, metadata={"description": __("Database name")} ) query = fields.Dict( keys=fields.Str(), @@ -244,7 +245,7 @@ def build_sqlalchemy_uri( # pylint: disable=unused-argument if database or token: database = "md:" + database - return MDURI(token, database, token, query) + return str(MDURI(token, database, token, query)) @classmethod def get_parameters_from_uri( # pylint: disable=unused-argument From f3cd6cfd4cd8ae448b94441d2fbde365eec79c43 Mon Sep 17 00:00:00 2001 From: Guen Prawiroatmodjo Date: Thu, 4 Apr 2024 11:36:57 -0700 Subject: [PATCH 05/19] indentation --- .../Form/LabeledErrorBoundInput.tsx | 20 ++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) diff --git a/superset-frontend/src/components/Form/LabeledErrorBoundInput.tsx b/superset-frontend/src/components/Form/LabeledErrorBoundInput.tsx index 5c80120058db9..e393486960c81 100644 --- a/superset-frontend/src/components/Form/LabeledErrorBoundInput.tsx +++ b/superset-frontend/src/components/Form/LabeledErrorBoundInput.tsx @@ -150,13 +150,19 @@ const LabeledErrorBoundInput = ({ )} {props.name === 'access_token' ? ( - + ) : (
)} From 9c8670863765da80273160d78bab20fade72d7f9 Mon Sep 17 00:00:00 2001 From: Guen Prawiroatmodjo Date: Thu, 4 Apr 2024 11:43:36 -0700 Subject: [PATCH 06/19] remove unused code --- superset/databases/utils.py | 5 ----- 1 file changed, 5 deletions(-) diff --git a/superset/databases/utils.py b/superset/databases/utils.py index 5d37a5f402654..e4ee4429372ac 100644 --- a/superset/databases/utils.py +++ b/superset/databases/utils.py @@ -117,11 +117,6 @@ def make_url_safe(raw_url: Union[str, URL]) -> Any: url = raw_url.strip() try: return make_url(url) # noqa - except ValueError: - if "duckdb" in url: - from superset.db_engine_specs.duckdb import MDURI - return MDURI.from_str(url) - raise DatabaseInvalidError() except Exception: raise DatabaseInvalidError() # pylint: disable=raise-missing-from From 9b77d30616dcdae0889a3d2bee11bd451f7854dd Mon Sep 17 00:00:00 2001 From: Guen Prawiroatmodjo Date: Thu, 4 Apr 2024 16:27:20 -0700 Subject: [PATCH 07/19] move hard-coded url to duckdb engine spec --- .../Form/LabeledErrorBoundInput.tsx | 5 +- .../CommonParameters.tsx | 2 + .../DatabaseConnectionForm/index.tsx | 1 + .../databases/DatabaseModal/ModalHeader.tsx | 3 +- .../src/features/databases/types.ts | 1 + superset/db_engine_specs/duckdb.py | 117 ++++-------------- 6 files changed, 31 insertions(+), 98 deletions(-) diff --git a/superset-frontend/src/components/Form/LabeledErrorBoundInput.tsx b/superset-frontend/src/components/Form/LabeledErrorBoundInput.tsx index e393486960c81..33f7f2cf5e92b 100644 --- a/superset-frontend/src/components/Form/LabeledErrorBoundInput.tsx +++ b/superset-frontend/src/components/Form/LabeledErrorBoundInput.tsx @@ -109,6 +109,7 @@ const LabeledErrorBoundInput = ({ id, className, visibilityToggle, + get_url, ...props }: LabeledErrorBoundInputProps) => ( @@ -154,10 +155,8 @@ const LabeledErrorBoundInput = ({ type="link" htmlType="button" onClick={() => { - const href = - 'https://app.motherduck.com/token-request?appName=Superset&close=y'; + const href = get_url; window.open(href); - console.log(props); return true; }} > diff --git a/superset-frontend/src/features/databases/DatabaseModal/DatabaseConnectionForm/CommonParameters.tsx b/superset-frontend/src/features/databases/DatabaseModal/DatabaseConnectionForm/CommonParameters.tsx index 2c95dc9e768e2..1aea720da233e 100644 --- a/superset-frontend/src/features/databases/DatabaseModal/DatabaseConnectionForm/CommonParameters.tsx +++ b/superset-frontend/src/features/databases/DatabaseModal/DatabaseConnectionForm/CommonParameters.tsx @@ -163,6 +163,7 @@ export const accessTokenField = ({ validationErrors, db, isEditMode, + default_value, }: FieldPropTypes) => ( diff --git a/superset-frontend/src/features/databases/DatabaseModal/DatabaseConnectionForm/index.tsx b/superset-frontend/src/features/databases/DatabaseModal/DatabaseConnectionForm/index.tsx index fc076b624f0e8..d3d1d81b5c9fa 100644 --- a/superset-frontend/src/features/databases/DatabaseModal/DatabaseConnectionForm/index.tsx +++ b/superset-frontend/src/features/databases/DatabaseModal/DatabaseConnectionForm/index.tsx @@ -169,6 +169,7 @@ const DatabaseConnectionForm = ({ db, key: field, field, + default_value: parameters.properties[field]?.default, isEditMode, sslForced, editNewDb, diff --git a/superset-frontend/src/features/databases/DatabaseModal/ModalHeader.tsx b/superset-frontend/src/features/databases/DatabaseModal/ModalHeader.tsx index d6daa2ca00330..6245edb145280 100644 --- a/superset-frontend/src/features/databases/DatabaseModal/ModalHeader.tsx +++ b/superset-frontend/src/features/databases/DatabaseModal/ModalHeader.tsx @@ -149,8 +149,7 @@ const ModalHeader = ({ target="_blank" rel="noopener noreferrer" > - {t('connecting to %(dbModelName)s', { dbModelName: dbModel.name })} - . + {t('connecting to %(dbModelName)s', { dbModelName: dbModel.name })}.

diff --git a/superset-frontend/src/features/databases/types.ts b/superset-frontend/src/features/databases/types.ts index 2dff61d5e8a35..e6f5291a3cb2a 100644 --- a/superset-frontend/src/features/databases/types.ts +++ b/superset-frontend/src/features/databases/types.ts @@ -291,6 +291,7 @@ export interface FieldPropTypes { db?: DatabaseObject; dbModel?: DatabaseForm; field: string; + default_value?: any; isEditMode?: boolean; sslForced?: boolean; defaultDBName?: string; diff --git a/superset/db_engine_specs/duckdb.py b/superset/db_engine_specs/duckdb.py index 7fbc6bcee6751..62e80d6f369a5 100644 --- a/superset/db_engine_specs/duckdb.py +++ b/superset/db_engine_specs/duckdb.py @@ -19,31 +19,21 @@ import re from datetime import datetime from re import Pattern -from typing import Any, TypedDict, TYPE_CHECKING - -from apispec import APISpec -from apispec.ext.marshmallow import MarshmallowPlugin -from flask_babel import gettext as __ -from marshmallow import fields, Schema -from sqlalchemy import types -from sqlalchemy.engine.reflection import Inspector -from sqlalchemy.engine.url import URL - -from superset.config import VERSION_STRING -from superset.constants import TimeGrain, USER_AGENT, PASSWORD_MASK -from superset.db_engine_specs.base import BaseEngineSpec -from superset.errors import SupersetErrorType -from superset.errors import SupersetError +from typing import Any, TYPE_CHECKING, TypedDict from apispec import APISpec from apispec.ext.marshmallow import MarshmallowPlugin from flask_babel import gettext as __, lazy_gettext as _ from marshmallow import fields, Schema from marshmallow.validate import Range +from sqlalchemy import types, util from sqlalchemy.engine.reflection import Inspector from sqlalchemy.engine.url import URL -from sqlalchemy import util + +from superset.config import VERSION_STRING +from superset.constants import PASSWORD_MASK, TimeGrain, USER_AGENT from superset.databases.utils import make_url_safe +from superset.db_engine_specs.base import BaseEngineSpec from superset.errors import ErrorLevel, SupersetError, SupersetErrorType from superset.utils.network import is_hostname_valid, is_port_open @@ -55,18 +45,20 @@ COLUMN_DOES_NOT_EXIST_REGEX = re.compile("no such column: (?P.+)") -class MDURI(util.namedtuple( - "MDURI", +class DuckDBURI( + util.namedtuple( # type: ignore + "DuckDBURI", [ "password", "database", "motherduck_token", "query", ], - )): + ) +): drivername = "duckdb" - def __str__(self): + def __str__(self) -> str: query = "" if self.motherduck_token: query += f"?motherduck_token={self.motherduck_token}" @@ -75,9 +67,8 @@ def __str__(self): query += "&".join([f"{key}={value}" for key, value in self.query.items()]) return f"{DuckDBEngineSpec.engine}:///{self.database}{query}" - @classmethod - def from_str(cls, name: str): + def from_str(cls, name: str) -> DuckDBURI: pattern = re.compile( r""" (?P[\w\+]+):/// @@ -88,11 +79,10 @@ def from_str(cls, name: str): """, re.X, ) - m = pattern.match(name) - if m is not None: + if (m := pattern.match(name)) is not None: components = m.groupdict() if components["query"] is not None: - query = {} + query: Any = {} for key, value in util.parse_qsl(components["query"]): if util.py2k: @@ -115,78 +105,19 @@ def from_str(cls, name: str): password=token, database=components.get("database"), motherduck_token=token, - query=query + query=query, ) + else: + raise ValueError(f"Connection string {name} is not a valid DuckDB URI") - def strip(self): - return self - - def set( - self, - password=None, - database=None, - query=None, - ): - """return a new :class:`_engine.URL` object with modifications. - - Values are used if they are non-None. To set a value to ``None`` - explicitly, use the :meth:`MDURI._replace` method adapted - from ``namedtuple``. - - :param password: new password - :param database: new database - :param query: new query parameters, passed a dict of string keys - referring to string or sequence of string values. Fully - replaces the previous list of arguments. - - :return: new :class:`_engine.URL` object. - - .. versionadded:: 1.4 - - .. seealso:: - - :meth:`_engine.URL.update_query_dict` - - """ - - kw = {} - if password is not None: - kw["password"] = password - if database is not None: - kw["database"] = database - if query is not None: - kw["query"] = query - - return self._replace(**kw) - - def get_backend_name(self): - """Return the backend name. - - This is the name that corresponds to the database backend in - use, and is the portion of the :attr:`_engine.URL.drivername` - that is to the left of the plus sign. - - """ - return self.drivername - - def get_driver_name(self): - """Return the backend name. - - This is the name that corresponds to the DBAPI driver in - use, and is the portion of the :attr:`_engine.URL.drivername` - that is to the right of the plus sign. - - If the :attr:`_engine.URL.drivername` does not include a plus sign, - then the default :class:`_engine.Dialect` for this :class:`_engine.URL` - is imported in order to get the driver name. - - """ - return self.drivername # schema for adding a database by providing parameters instead of the # full SQLAlchemy URI class DuckDBParametersSchema(Schema): - access_token = fields.String(allow_none=True, metadata={"description": __("MotherDuck token")} + access_token = fields.String( + allow_none=True, + metadata={"description": __("MotherDuck token")}, + load_default="https://app.motherduck.com/token-request?appName=Superset&close=y", ) database = fields.String( required=False, metadata={"description": __("Database name")} @@ -245,8 +176,8 @@ def build_sqlalchemy_uri( # pylint: disable=unused-argument if database or token: database = "md:" + database - return str(MDURI(token, database, token, query)) - + return str(DuckDBURI(token, database, token, query)) + @classmethod def get_parameters_from_uri( # pylint: disable=unused-argument cls, uri: str, encrypted_extra: dict[str, Any] | None = None From 6c53983974c11c18eacce6c7e94a6642ab0964ba Mon Sep 17 00:00:00 2001 From: Guen Prawiroatmodjo Date: Thu, 4 Apr 2024 16:41:42 -0700 Subject: [PATCH 08/19] dynamically set link text from parameters via description --- .../src/components/Form/LabeledErrorBoundInput.tsx | 8 ++++---- .../DatabaseConnectionForm/CommonParameters.tsx | 8 +++++++- .../DatabaseModal/DatabaseConnectionForm/index.tsx | 1 + superset-frontend/src/features/databases/types.ts | 1 + 4 files changed, 13 insertions(+), 5 deletions(-) diff --git a/superset-frontend/src/components/Form/LabeledErrorBoundInput.tsx b/superset-frontend/src/components/Form/LabeledErrorBoundInput.tsx index 33f7f2cf5e92b..d2988a87ce1fc 100644 --- a/superset-frontend/src/components/Form/LabeledErrorBoundInput.tsx +++ b/superset-frontend/src/components/Form/LabeledErrorBoundInput.tsx @@ -110,6 +110,7 @@ const LabeledErrorBoundInput = ({ className, visibilityToggle, get_url, + description, ...props }: LabeledErrorBoundInputProps) => ( @@ -150,17 +151,16 @@ const LabeledErrorBoundInput = ({ ) : ( )} - {props.name === 'access_token' ? ( + {get_url && description ? ( ) : (
diff --git a/superset-frontend/src/features/databases/DatabaseModal/DatabaseConnectionForm/CommonParameters.tsx b/superset-frontend/src/features/databases/DatabaseModal/DatabaseConnectionForm/CommonParameters.tsx index 1aea720da233e..529fc18419e4c 100644 --- a/superset-frontend/src/features/databases/DatabaseModal/DatabaseConnectionForm/CommonParameters.tsx +++ b/superset-frontend/src/features/databases/DatabaseModal/DatabaseConnectionForm/CommonParameters.tsx @@ -164,6 +164,7 @@ export const accessTokenField = ({ db, isEditMode, default_value, + description, }: FieldPropTypes) => ( diff --git a/superset-frontend/src/features/databases/DatabaseModal/DatabaseConnectionForm/index.tsx b/superset-frontend/src/features/databases/DatabaseModal/DatabaseConnectionForm/index.tsx index d3d1d81b5c9fa..509103ea29fe5 100644 --- a/superset-frontend/src/features/databases/DatabaseModal/DatabaseConnectionForm/index.tsx +++ b/superset-frontend/src/features/databases/DatabaseModal/DatabaseConnectionForm/index.tsx @@ -170,6 +170,7 @@ const DatabaseConnectionForm = ({ key: field, field, default_value: parameters.properties[field]?.default, + description: parameters.properties[field]?.description, isEditMode, sslForced, editNewDb, diff --git a/superset-frontend/src/features/databases/types.ts b/superset-frontend/src/features/databases/types.ts index e6f5291a3cb2a..06799ebaface1 100644 --- a/superset-frontend/src/features/databases/types.ts +++ b/superset-frontend/src/features/databases/types.ts @@ -292,6 +292,7 @@ export interface FieldPropTypes { dbModel?: DatabaseForm; field: string; default_value?: any; + description?: string; isEditMode?: boolean; sslForced?: boolean; defaultDBName?: string; From 05cd528d86be5fc35e9b5edec89b15f00194f688 Mon Sep 17 00:00:00 2001 From: Guen Prawiroatmodjo Date: Tue, 9 Apr 2024 13:20:01 -0700 Subject: [PATCH 09/19] Update superset/databases/utils.py Co-authored-by: Beto Dealmeida --- superset/databases/utils.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/superset/databases/utils.py b/superset/databases/utils.py index e4ee4429372ac..21abd7b9c283d 100644 --- a/superset/databases/utils.py +++ b/superset/databases/utils.py @@ -104,7 +104,7 @@ def get_table_metadata( } -def make_url_safe(raw_url: Union[str, URL]) -> Any: +def make_url_safe(raw_url: Union[str, URL]) -> URL: """ Wrapper for SQLAlchemy's make_url(), which tends to raise too detailed of errors, which inevitably find their way into server logs. ArgumentErrors From d68f0491069213b058155a6105dbaf226133395c Mon Sep 17 00:00:00 2001 From: Guen Prawiroatmodjo Date: Tue, 9 Apr 2024 13:20:32 -0700 Subject: [PATCH 10/19] Update superset/db_engine_specs/duckdb.py Co-authored-by: Beto Dealmeida --- superset/db_engine_specs/duckdb.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/superset/db_engine_specs/duckdb.py b/superset/db_engine_specs/duckdb.py index 62e80d6f369a5..546be5731fa9f 100644 --- a/superset/db_engine_specs/duckdb.py +++ b/superset/db_engine_specs/duckdb.py @@ -203,9 +203,6 @@ def validate_parameters( ) -> 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] = [] From 0542c58b837194f5e9032445beee0dff84e39bf4 Mon Sep 17 00:00:00 2001 From: Guen Prawiroatmodjo Date: Tue, 9 Apr 2024 16:57:43 -0700 Subject: [PATCH 11/19] refactor sqlalchemy uri build and remove unused code --- superset/db_engine_specs/duckdb.py | 120 ++++++++++------------------- 1 file changed, 41 insertions(+), 79 deletions(-) diff --git a/superset/db_engine_specs/duckdb.py b/superset/db_engine_specs/duckdb.py index 546be5731fa9f..a22fafd1985cd 100644 --- a/superset/db_engine_specs/duckdb.py +++ b/superset/db_engine_specs/duckdb.py @@ -16,6 +16,7 @@ # under the License. from __future__ import annotations +import os import re from datetime import datetime from re import Pattern @@ -43,72 +44,7 @@ COLUMN_DOES_NOT_EXIST_REGEX = re.compile("no such column: (?P.+)") - - -class DuckDBURI( - util.namedtuple( # type: ignore - "DuckDBURI", - [ - "password", - "database", - "motherduck_token", - "query", - ], - ) -): - drivername = "duckdb" - - def __str__(self) -> str: - query = "" - if self.motherduck_token: - query += f"?motherduck_token={self.motherduck_token}" - else: - query += "?" - query += "&".join([f"{key}={value}" for key, value in self.query.items()]) - return f"{DuckDBEngineSpec.engine}:///{self.database}{query}" - - @classmethod - def from_str(cls, name: str) -> DuckDBURI: - pattern = re.compile( - r""" - (?P[\w\+]+):/// - (?: - (md:(?P[^/\?]*))? - )? - (?:\?(?P.*))? - """, - re.X, - ) - if (m := pattern.match(name)) is not None: - components = m.groupdict() - if components["query"] is not None: - query: Any = {} - - for key, value in util.parse_qsl(components["query"]): - if util.py2k: - key = key.encode("ascii") - if key in query: - query[key] = util.to_list(query[key]) - query[key].append(value) - else: - query[key] = value - else: - query = None - if query is not None: - token = query.pop("motherduck_token") - else: - token = None - - components["query"] = query - - return cls( - password=token, - database=components.get("database"), - motherduck_token=token, - query=query, - ) - else: - raise ValueError(f"Connection string {name} is not a valid DuckDB URI") +DEFAULT_ACCESS_TOKEN_URL = "https://app.motherduck.com/token-request?appName=Superset&close=y" # schema for adding a database by providing parameters instead of the @@ -117,7 +53,7 @@ class DuckDBParametersSchema(Schema): access_token = fields.String( allow_none=True, metadata={"description": __("MotherDuck token")}, - load_default="https://app.motherduck.com/token-request?appName=Superset&close=y", + load_default=DEFAULT_ACCESS_TOKEN_URL, ) database = fields.String( required=False, metadata={"description": __("Database name")} @@ -162,21 +98,24 @@ class DuckDBParametersMixin: # for Postgres this would be `{"sslmode": "verify-ca"}`, eg. encryption_parameters: dict[str, str] = {} + @staticmethod + def _is_motherduck(database: str) -> bool: + return "md:" in database + @classmethod def build_sqlalchemy_uri( # pylint: disable=unused-argument cls, parameters: DuckDBParametersType, encrypted_extra: dict[str, str] | None = None, ) -> str: - # make a copy so that we don't update the original - query = parameters.get("query", {}).copy() - token = parameters.get("access_token", "") + query = parameters.get("query", {}) database = parameters.get("database", "") + token = parameters.get("access_token") - if database or token: - database = "md:" + database + if cls._is_motherduck(database) or (token and token != DEFAULT_ACCESS_TOKEN_URL): + return MotherDuckEngineSpec.build_sqlalchemy_uri(parameters) - return str(DuckDBURI(token, database, token, query)) + return str(URL(drivername=cls.engine, database=database, query=query)) @classmethod def get_parameters_from_uri( # pylint: disable=unused-argument @@ -188,11 +127,9 @@ def get_parameters_from_uri( # pylint: disable=unused-argument for (key, value) in url.query.items() if (key, value) not in cls.encryption_parameters.items() } - encryption = all( - item in url.query.items() for item in cls.encryption_parameters.items() - ) + access_token = query.pop("motherduck_token", "") return { - "access_token": url.access_token, + "access_token": access_token, "database": url.database, "query": query, } @@ -206,8 +143,11 @@ def validate_parameters( """ errors: list[SupersetError] = [] - required = {"access_token"} parameters = properties.get("parameters", {}) + if cls._is_motherduck(parameters.get("database", "")): + required = {"access_token"} + else: + required = set() present = {key for key in parameters if parameters.get(key, ())} if missing := sorted(required - present): @@ -306,9 +246,31 @@ def get_extra_params(database: Database) -> dict[str, Any]: class MotherDuckEngineSpec(DuckDBEngineSpec): - engine = "duckdb" + engine = "motherduck" engine_name = "MotherDuck" + engine_aliases: set[str] = {"duckdb"} sqlalchemy_uri_placeholder = ( "duckdb:///md:{database_name}?motherduck_token={SERVICE_TOKEN}" ) + + @staticmethod + def _is_motherduck(database: str) -> bool: + return True + + @classmethod + def build_sqlalchemy_uri( # pylint: disable=unused-argument + cls, + parameters: DuckDBParametersType, + encrypted_extra: dict[str, str] | None = None, + ) -> str: + # make a copy so that we don't update the original + query = parameters.get("query", {}).copy() + database = parameters.get("database", "") + token = parameters.get("access_token") + + if not database.startswith("md:"): + database = f"md:{database}" + query["motherduck_token"] = token + + return str(URL(drivername=DuckDBEngineSpec.engine, database=database, query=query)) From 3763eefb11d7c13bc47c367b3213c8c71c0f98b7 Mon Sep 17 00:00:00 2001 From: Guen Prawiroatmodjo Date: Tue, 9 Apr 2024 17:04:07 -0700 Subject: [PATCH 12/19] formatting --- superset/db_engine_specs/duckdb.py | 23 ++++++++++++++--------- 1 file changed, 14 insertions(+), 9 deletions(-) diff --git a/superset/db_engine_specs/duckdb.py b/superset/db_engine_specs/duckdb.py index a22fafd1985cd..9b02e57b3cf2f 100644 --- a/superset/db_engine_specs/duckdb.py +++ b/superset/db_engine_specs/duckdb.py @@ -16,7 +16,6 @@ # under the License. from __future__ import annotations -import os import re from datetime import datetime from re import Pattern @@ -26,17 +25,15 @@ from apispec.ext.marshmallow import MarshmallowPlugin from flask_babel import gettext as __, lazy_gettext as _ from marshmallow import fields, Schema -from marshmallow.validate import Range -from sqlalchemy import types, util +from sqlalchemy import types from sqlalchemy.engine.reflection import Inspector from sqlalchemy.engine.url import URL from superset.config import VERSION_STRING -from superset.constants import PASSWORD_MASK, TimeGrain, USER_AGENT +from superset.constants import TimeGrain, USER_AGENT from superset.databases.utils import make_url_safe from superset.db_engine_specs.base import BaseEngineSpec from superset.errors import ErrorLevel, SupersetError, SupersetErrorType -from superset.utils.network import is_hostname_valid, is_port_open if TYPE_CHECKING: # prevent circular imports @@ -44,7 +41,9 @@ COLUMN_DOES_NOT_EXIST_REGEX = re.compile("no such column: (?P.+)") -DEFAULT_ACCESS_TOKEN_URL = "https://app.motherduck.com/token-request?appName=Superset&close=y" +DEFAULT_ACCESS_TOKEN_URL = ( + "https://app.motherduck.com/token-request?appName=Superset&close=y" +) # schema for adding a database by providing parameters instead of the @@ -88,6 +87,8 @@ class DuckDBParametersMixin: """ + engine = "duckdb" + # schema describing the parameters used to configure the DB parameters_schema = DuckDBParametersSchema() @@ -112,7 +113,9 @@ def build_sqlalchemy_uri( # pylint: disable=unused-argument database = parameters.get("database", "") token = parameters.get("access_token") - if cls._is_motherduck(database) or (token and token != DEFAULT_ACCESS_TOKEN_URL): + if cls._is_motherduck(database) or ( + token and token != DEFAULT_ACCESS_TOKEN_URL + ): return MotherDuckEngineSpec.build_sqlalchemy_uri(parameters) return str(URL(drivername=cls.engine, database=database, query=query)) @@ -259,7 +262,7 @@ def _is_motherduck(database: str) -> bool: return True @classmethod - def build_sqlalchemy_uri( # pylint: disable=unused-argument + def build_sqlalchemy_uri( cls, parameters: DuckDBParametersType, encrypted_extra: dict[str, str] | None = None, @@ -273,4 +276,6 @@ def build_sqlalchemy_uri( # pylint: disable=unused-argument database = f"md:{database}" query["motherduck_token"] = token - return str(URL(drivername=DuckDBEngineSpec.engine, database=database, query=query)) + return str( + URL(drivername=DuckDBEngineSpec.engine, database=database, query=query) + ) From 403111c5db596c44b1b84cda816d6d96bc571cb3 Mon Sep 17 00:00:00 2001 From: Guen Prawiroatmodjo Date: Tue, 9 Apr 2024 17:06:51 -0700 Subject: [PATCH 13/19] set token to empty string by default --- superset/db_engine_specs/duckdb.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/superset/db_engine_specs/duckdb.py b/superset/db_engine_specs/duckdb.py index 9b02e57b3cf2f..ee26180df867e 100644 --- a/superset/db_engine_specs/duckdb.py +++ b/superset/db_engine_specs/duckdb.py @@ -270,7 +270,7 @@ def build_sqlalchemy_uri( # make a copy so that we don't update the original query = parameters.get("query", {}).copy() database = parameters.get("database", "") - token = parameters.get("access_token") + token = parameters.get("access_token", "") if not database.startswith("md:"): database = f"md:{database}" From 8829806c13fe1d53d2f2a234218a8ebae14ac916 Mon Sep 17 00:00:00 2001 From: Guen Prawiroatmodjo Date: Tue, 9 Apr 2024 17:41:41 -0700 Subject: [PATCH 14/19] throw error if no token is provided for motherduck engine --- superset/db_engine_specs/duckdb.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/superset/db_engine_specs/duckdb.py b/superset/db_engine_specs/duckdb.py index ee26180df867e..36b5d8786c96a 100644 --- a/superset/db_engine_specs/duckdb.py +++ b/superset/db_engine_specs/duckdb.py @@ -274,7 +274,10 @@ def build_sqlalchemy_uri( if not database.startswith("md:"): database = f"md:{database}" - query["motherduck_token"] = token + if token and token != DEFAULT_ACCESS_TOKEN_URL: + query["motherduck_token"] = token + else: + raise ValueError(f"Need MotherDuck token to connect to database '{database}'.") return str( URL(drivername=DuckDBEngineSpec.engine, database=database, query=query) From 2663e61a1a82f6aa7f58b6d6fcc4f9cfc8d0382e Mon Sep 17 00:00:00 2001 From: Guen Prawiroatmodjo Date: Tue, 9 Apr 2024 17:42:13 -0700 Subject: [PATCH 15/19] formatting --- superset/db_engine_specs/duckdb.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/superset/db_engine_specs/duckdb.py b/superset/db_engine_specs/duckdb.py index 36b5d8786c96a..74f1a071575b9 100644 --- a/superset/db_engine_specs/duckdb.py +++ b/superset/db_engine_specs/duckdb.py @@ -277,7 +277,9 @@ def build_sqlalchemy_uri( if token and token != DEFAULT_ACCESS_TOKEN_URL: query["motherduck_token"] = token else: - raise ValueError(f"Need MotherDuck token to connect to database '{database}'.") + raise ValueError( + f"Need MotherDuck token to connect to database '{database}'." + ) return str( URL(drivername=DuckDBEngineSpec.engine, database=database, query=query) From 6430dd25fcf835f06aed32987a1a0866a60d1810 Mon Sep 17 00:00:00 2001 From: Guen Prawiroatmodjo Date: Tue, 9 Apr 2024 18:17:34 -0700 Subject: [PATCH 16/19] add UTs, return :memory: as default db for duckdb engine --- superset/db_engine_specs/duckdb.py | 2 +- .../unit_tests/db_engine_specs/test_duckdb.py | 53 +++++++++++++++++++ 2 files changed, 54 insertions(+), 1 deletion(-) diff --git a/superset/db_engine_specs/duckdb.py b/superset/db_engine_specs/duckdb.py index 74f1a071575b9..1aa147cea83e3 100644 --- a/superset/db_engine_specs/duckdb.py +++ b/superset/db_engine_specs/duckdb.py @@ -110,7 +110,7 @@ def build_sqlalchemy_uri( # pylint: disable=unused-argument encrypted_extra: dict[str, str] | None = None, ) -> str: query = parameters.get("query", {}) - database = parameters.get("database", "") + database = parameters.get("database", ":memory:") token = parameters.get("access_token") if cls._is_motherduck(database) or ( diff --git a/tests/unit_tests/db_engine_specs/test_duckdb.py b/tests/unit_tests/db_engine_specs/test_duckdb.py index 39c70470f4ede..1e33522def4af 100644 --- a/tests/unit_tests/db_engine_specs/test_duckdb.py +++ b/tests/unit_tests/db_engine_specs/test_duckdb.py @@ -72,3 +72,56 @@ def test_get_extra_params(mocker: MockerFixture) -> None: } } } + + +def test_build_sqlalchemy_uri() -> None: + """Test DuckDBEngineSpec.build_sqlalchemy_uri""" + from superset.db_engine_specs.duckdb import DuckDBEngineSpec, DuckDBParametersType + + # No database provided, default to :memory: + parameters = DuckDBParametersType() + uri = DuckDBEngineSpec.build_sqlalchemy_uri(parameters) + assert "duckdb:///:memory:" == uri + + # Database provided + parameters = DuckDBParametersType(database="/path/to/duck.db") + uri = DuckDBEngineSpec.build_sqlalchemy_uri(parameters) + assert "duckdb:////path/to/duck.db" == uri + + +def test_md_build_sqlalchemy_uri() -> None: + """Test MotherDuckEngineSpec.build_sqlalchemy_uri""" + from superset.db_engine_specs.duckdb import ( + DuckDBParametersType, + MotherDuckEngineSpec, + ) + + # No access token provided, throw ValueError + parameters = DuckDBParametersType(database="my_db") + with pytest.raises(ValueError): + MotherDuckEngineSpec.build_sqlalchemy_uri(parameters) + + # No database provided, default to "md:" + parameters = DuckDBParametersType(access_token="token") + uri = MotherDuckEngineSpec.build_sqlalchemy_uri(parameters) + assert "duckdb:///md:?motherduck_token=token" + + # Database and access_token provided + parameters = DuckDBParametersType(database="my_db", access_token="token") + uri = MotherDuckEngineSpec.build_sqlalchemy_uri(parameters) + assert "duckdb:///md:my_db?motherduck_token=token" == uri + + +def test_get_parameters_from_uri() -> None: + from superset.db_engine_specs.duckdb import DuckDBEngineSpec + + uri = "duckdb:////path/to/duck.db" + parameters = DuckDBEngineSpec.get_parameters_from_uri(uri) + + assert parameters["database"] == "/path/to/duck.db" + + uri = "duckdb:///md:my_db?motherduck_token=token" + parameters = DuckDBEngineSpec.get_parameters_from_uri(uri) + + assert parameters["database"] == "md:my_db" + assert parameters["access_token"] == "token" From 9eebe8310900dc79613f2146dbc365fcfe6d73b2 Mon Sep 17 00:00:00 2001 From: Guen Prawiroatmodjo Date: Tue, 9 Apr 2024 18:31:25 -0700 Subject: [PATCH 17/19] use the Superset default button instead of bare antd version --- .../src/components/Form/LabeledErrorBoundInput.tsx | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/superset-frontend/src/components/Form/LabeledErrorBoundInput.tsx b/superset-frontend/src/components/Form/LabeledErrorBoundInput.tsx index d2988a87ce1fc..c882511395547 100644 --- a/superset-frontend/src/components/Form/LabeledErrorBoundInput.tsx +++ b/superset-frontend/src/components/Form/LabeledErrorBoundInput.tsx @@ -17,10 +17,11 @@ * under the License. */ import React from 'react'; -import { Input, Tooltip, Button } from 'antd'; +import { Input, Tooltip } from 'antd'; import { styled, css, SupersetTheme, t } from '@superset-ui/core'; import InfoTooltip from 'src/components/InfoTooltip'; import Icons from 'src/components/Icons'; +import Button from 'src/components/Button'; import errorIcon from 'src/assets/images/icons/error.svg'; import FormItem from './FormItem'; import FormLabel from './FormLabel'; @@ -155,6 +156,7 @@ const LabeledErrorBoundInput = ({