FrancescoCastaldi commented on code in PR #43695:
URL: https://github.com/apache/superset/pull/43695#discussion_r4258862391
##########
superset/db_engine_specs/databricks.py:
##########
@@ -629,9 +657,22 @@ class
DatabricksNativeEngineSpec(DatabricksDynamicBaseEngineSpec):
"databricks+connector://token:{access_token}@{host}:{port}/{database_name}"
)
- # Note: Primary metadata is in DatabricksPythonConnectorEngineSpec which
- # consolidates all Databricks connection methods. This spec exists for
- # backwards compatibility with legacy databricks-dbapi connections.
+ metadata = {
+ "description": ("Databricks legacy connector using databricks-dbapi."),
+ "logo": "databricks.png",
+ "homepage_url": "https://www.databricks.com/";,
+ "categories": [
+ DatabaseCategory.CLOUD_DATA_WAREHOUSES,
+ DatabaseCategory.ANALYTICAL_DATABASES,
+ DatabaseCategory.HOSTED_OPEN_SOURCE,
+ ],
+ "pypi_packages": ["databricks-dbapi[sqlalchemy]"],
Review Comment:
Implemented adjust_engine_params on DatabricksNativeEngineSpec to translate
catalog and schema into query parameters (?catalog= and ?schema=) and forward
http_path from connect_args when present. Updated test_databricks.py to build
the URI with DatabricksNativeEngineSpec.build_sqlalchemy_uri and exercise
adjust_engine_params, verifying that catalog and schema are extracted properly
by DatabricksDialect.
##########
superset/db_engine_specs/databricks.py:
##########
@@ -955,6 +980,29 @@ class DatabricksODBCEngineSpec(DatabricksBaseEngineSpec):
drivers = {"pyodbc": "ODBC driver for SQL endpoint"}
default_driver = "pyodbc"
+ metadata = {
+ "description": ("Databricks SQL Endpoint connectivity via the pyodbc
driver."),
+ "logo": "databricks.png",
+ "homepage_url": "https://www.databricks.com/";,
+ "categories": [
+ DatabaseCategory.CLOUD_DATA_WAREHOUSES,
+ DatabaseCategory.ANALYTICAL_DATABASES,
+ DatabaseCategory.PROPRIETARY,
+ ],
+ "pypi_packages": ["pyodbc"],
Review Comment:
Updated pypi_packages to ["databricks-dbapi[odbc]", "pyodbc"] and documented
the SQLAlchemy 2.0 limitation under known_incompatibilities (following the
Kusto spec pattern) to make clear that databricks-dbapi pins SQLAlchemy below
2.0 and that the Databricks Python Connector (databricks-sql-connector) is the
recommended path forward.
##########
superset/db_engine_specs/databricks.py:
##########
@@ -974,6 +1022,31 @@ class DatabricksHiveEngineSpec(HiveEngineSpec):
drivers = {"pyhive": "Hive driver for Interactive Cluster"}
default_driver = "pyhive"
+ metadata = {
+ "description": (
+ "Databricks Interactive Cluster connectivity via the PyHive
connector."
+ ),
+ "logo": "databricks.png",
+ "homepage_url": "https://www.databricks.com/";,
+ "categories": [
+ DatabaseCategory.CLOUD_DATA_WAREHOUSES,
+ DatabaseCategory.ANALYTICAL_DATABASES,
+ DatabaseCategory.HOSTED_OPEN_SOURCE,
+ ],
+ "pypi_packages": ["pyhive"],
Review Comment:
Updated pypi_packages to ["databricks-dbapi[hive]", "pyhive", "thrift"] and
added a matching known_incompatibilities entry for SQLAlchemy 2.0, pointing
users to databricks-sql-connector for modern deployments.
##########
tests/unit_tests/db_engine_specs/test_arc.py:
##########
@@ -0,0 +1,57 @@
+# 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.
+import pytest
+
+from superset.constants import TimeGrain
+from superset.db_engine_specs.arc import ArcEngineSpec
+
+
+def test_arc_properties() -> None:
+ assert ArcEngineSpec.engine == "arc"
+ assert ArcEngineSpec.engine_name == "Arc"
+ assert ArcEngineSpec.default_driver == "arrow"
+
+
+def test_arc_metadata() -> None:
+ metadata = ArcEngineSpec.metadata
+ assert (
+ metadata["description"]
+ == "Arc is a data platform with multiple connection options."
+ )
+ assert metadata["logo"] == "arc.png"
Review Comment:
Dropped the assert metadata["logo"] == ... assertions across test_arc.py,
test_ocient.py, test_odps.py, and test_parseable.py so the test suites pass
cleanly against the current metadata definitions.
##########
tests/unit_tests/db_engine_specs/test_teradata.py:
##########
@@ -0,0 +1,59 @@
+# 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.
+import pytest
+
+from superset.db_engine_specs.teradata import TeradataEngineSpec
+
+
+def test_teradata_properties() -> None:
+ assert TeradataEngineSpec.engine == "teradatasql"
+ assert TeradataEngineSpec.engine_name == "Teradata"
+ assert TeradataEngineSpec.max_column_name_length == 30
+
+
+def test_teradata_metadata() -> None:
+ metadata = TeradataEngineSpec.metadata
+ desc = metadata["description"]
+ assert "Teradata is an enterprise data warehouse platform" in desc
+ assert metadata["logo"] == "teradata.png"
+ assert "teradatasqlalchemy" in metadata["pypi_packages"]
+ assert metadata["default_port"] == 1025
+
+
+def test_teradata_epoch_to_dttm() -> None:
+ assert (
+ TeradataEngineSpec.epoch_to_dttm().format(col="ts")
+ == "CAST(((CAST(DATE '1970-01-01' + (ts / 86400) AS TIMESTAMP(0) AT
0)) AT 0) + ((ts MOD 86400) * INTERVAL '00:00:01' HOUR TO SECOND) AS
TIMESTAMP(0))"
Review Comment:
Split the expected SQL literal in test_teradata_epoch_to_dttm into adjacent
string literals within the 88-column Ruff limit while keeping the full
assertion intact.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
