sadpandajoe commented on code in PR #43695:
URL: https://github.com/apache/superset/pull/43695#discussion_r3985687428
##########
superset/db_engine_specs/ibmi.py:
##########
@@ -14,20 +14,43 @@
# 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}:{port}/{database}",
Review Comment:
The generated IBM i example includes `:{port}`, but `sqlalchemy-ibmi` turns
that into a `port` option that its driver allowlist rejects, so following this
URI raises `ValueError` before connecting. Could this keep the existing no-port
IBM i URI and omit `default_port` instead?
##########
superset/db_engine_specs/kusto.py:
##########
@@ -209,15 +209,41 @@ def convert_dttm(
class KustoKqlEngineSpec(BaseEngineSpec): # pylint: disable=abstract-method
- """Azure Data Explorer engine spec using native KQL query language.
-
- Note: Documentation is consolidated in KustoSqlEngineSpec (Azure Data
Explorer).
- This spec exists for runtime support of the kustokql driver.
- """
+ """Azure Data Explorer engine spec using native KQL query language."""
engine = "kustokql"
engine_name = "Azure Data Explorer (KQL)"
time_groupby_inline = True
+
+ metadata = {
+ "description": (
+ "Azure Data Explorer (Kusto) using native Kusto Query Language
(KQL) "
+ "for fast, exploratory telemetry and log analytics."
+ ),
+ "logo": "kusto.png",
+ "homepage_url":
"https://azure.microsoft.com/en-us/products/data-explorer/";,
+ "categories": [
+ DatabaseCategory.CLOUD_AZURE,
+ DatabaseCategory.ANALYTICAL_DATABASES,
+ DatabaseCategory.PROPRIETARY,
+ ],
+ "pypi_packages": ["sqlalchemy-kusto"],
Review Comment:
This KQL page advertises `sqlalchemy-kusto` without the sibling warning that
the package pins SQLAlchemy 1.4, while this project requires SQLAlchemy 2.0, so
the documented install cannot resolve. Could this metadata carry the same
`known_incompatibilities` entry as the SQL variant?
##########
superset/db_engine_specs/arc.py:
##########
@@ -42,9 +42,13 @@ class ArcEngineSpec(BaseEngineSpec):
metadata = {
"description": "Arc is a data platform with multiple connection
options.",
+ "logo": "arc.png",
+ "homepage_url": "https://arc.net";,
"categories": [DatabaseCategory.OTHER, DatabaseCategory.PROPRIETARY],
"pypi_packages": ["arc-superset-arrow"],
"connection_string":
("arc+arrow://{api_key}@{hostname}:{port}/{database}"),
+ "default_port": 443,
Review Comment:
The listed `arc-superset-arrow` dialect defaults to port 8000 and always
constructs an `http://` endpoint, so this default targets HTTP on 443 and
normally fails to connect. Could the metadata use the dialect's supported
default, or point to a driver configuration that actually serves this endpoint?
##########
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:
`databricks-dbapi[sqlalchemy]` registers only `databricks.pyhive` and
`databricks.pyodbc`, not the `databricks.connector` dialect advertised below,
and it requires SQLAlchemy 1.x while this project requires 2.0. Could this
metadata point to a package and scheme that can load under Superset's current
dependency range?
--
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]
