sadpandajoe commented on code in PR #43695:
URL: https://github.com/apache/superset/pull/43695#discussion_r4129327074
##########
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:
Installing only `pyodbc` won't register the `databricks+pyodbc` SQLAlchemy
dialect this connection string requires — that registration comes from
`databricks-dbapi[sqlalchemy]`. Following this metadata as documented raises
`NoSuchModuleError` before a connection is attempted. Could `pypi_packages`
list the package that actually registers this dialect instead of bare `pyodbc`?
##########
superset/db_engine_specs/ibmi.py:
##########
@@ -14,20 +14,41 @@
# KIND, either express or implied. See the License for the
# specific language governing permissions and limitations
# under the License.
+from superset.db_engine_specs.base import DatabaseCategory
+
from .db2 import Db2EngineSpec
class IBMiEngineSpec(Db2EngineSpec):
- """IBM Db2 for i (AS/400) engine spec.
-
- Note: Documentation is in Db2EngineSpec's compatible_databases section.
- This spec exists for runtime support of the ibmi driver.
- """
+ """IBM Db2 for i (AS/400) engine spec."""
engine = "ibmi"
engine_name = "IBM Db2 for i"
max_column_name_length = 128
+ metadata = {
+ "description": (
+ "IBM Db2 for i (formerly AS/400) is an integrated relational
database "
+ "engine on IBM Power systems running IBM i."
+ ),
+ "logo": "ibm-db2.svg",
+ "homepage_url": "https://www.ibm.com/products/db2-for-i";,
+ "categories": [
+ DatabaseCategory.TRADITIONAL_RDBMS,
+ DatabaseCategory.PROPRIETARY,
+ ],
+ "pypi_packages": ["sqlalchemy-ibmi"],
+ "connection_string": "ibmi://{username}:{password}@{host}/{database}",
+ "parameters": {
+ "username": "IBM i username",
+ "password": "IBM i password",
+ "host": "IBM i system host",
+ "database": "Library/schema name",
Review Comment:
This documents the `database` URI segment as "Library/schema name", but IBM
i's ODBC driver maps that segment to the RDB (relational database) name, not a
library or schema — a user who enters a library name there targets a
nonexistent RDB and fails to connect. Could the description say "RDB name"
instead?
##########
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:
Same issue as the ODBC spec above: installing only `pyhive` doesn't register
the `databricks+pyhive` SQLAlchemy dialect this connection string requires —
that comes from `databricks-dbapi[sqlalchemy]`. A user following this
metadata's package list alone hits `NoSuchModuleError`. Could `pypi_packages`
name the package that actually registers this dialect?
##########
superset/db_engine_specs/clickhouse.py:
##########
@@ -261,9 +261,25 @@ class ClickHouseEngineSpec(ClickHouseBaseEngineSpec):
_show_functions_column = "name"
supports_file_upload = False
- # Note: Primary metadata is in ClickHouseConnectEngineSpec which
consolidates
- # both drivers. This spec exists for backwards compatibility with existing
- # connections using the clickhouse-sqlalchemy driver.
+ metadata = {
+ "description": (
+ "ClickHouse is an open-source column-oriented database for
real-time "
+ "analytics using SQL (legacy clickhouse-sqlalchemy connector)."
+ ),
+ "logo": "clickhouse.png",
+ "homepage_url": "https://clickhouse.com/";,
+ "categories": [
+ DatabaseCategory.ANALYTICAL_DATABASES,
+ DatabaseCategory.OPEN_SOURCE,
+ ],
+ "pypi_packages": ["clickhouse-sqlalchemy"],
+ "connection_string": (
+ "clickhouse://{username}:{password}@{host}:{port}/{database}"
+ ),
+ "default_port": 8123,
+ "docs_url": "https://clickhouse.com/docs/";,
+ "sqlalchemy_docs_url":
"https://github.com/xzkostyan/clickhouse-sqlalchemy";,
Review Comment:
This adds `sqlalchemy_docs_url` (pointing at the legacy
`clickhouse-sqlalchemy` driver) to `ClickHouseEngineSpec`.
`ClickHouseConnectEngineSpec` — the recommended subclass — defines its own
`metadata` without this key, so a metadata reader that merges by inheritance
falls back to this legacy link for the recommended driver too. Could this key
be set explicitly on `ClickHouseConnectEngineSpec` as well so it doesn't
inherit the legacy driver's docs link?
##########
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:
This still lists `pypi_packages: ["databricks-dbapi[sqlalchemy]"]` with
`connection_string: "databricks+connector://..."` and `default_port: 443`.
`databricks-dbapi[sqlalchemy]` registers the
`databricks+pyhive`/`databricks+pyodbc` dialects, not `databricks+connector`,
and it requires SQLAlchemy <2.0 while this project requires 2.0. Following this
metadata as documented still raises a dependency conflict or
`NoSuchModuleError` before a connection is attempted. Could this point to a
package/scheme combination that actually loads under SQLAlchemy 2.0?
--
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]
