bito-code-review[bot] commented on code in PR #43566:
URL: https://github.com/apache/superset/pull/43566#discussion_r4068663075
##########
superset/db_engine_specs/aurora.py:
##########
@@ -36,6 +37,27 @@ class AuroraMySQLDataAPI(MySQLEngineSpec):
"region_name={region_name}"
)
+ metadata = {
+ "description": (
+ "Amazon Aurora MySQL accessed via the AWS Data API, allowing "
+ "database queries over HTTP without managing persistent
connections."
+ ),
+ "logo": "aws-aurora.jpg",
+ "homepage_url": "https://aws.amazon.com/rds/aurora/";,
+ "categories": [
+ DatabaseCategory.CLOUD_AWS,
+ DatabaseCategory.TRADITIONAL_RDBMS,
+ DatabaseCategory.HOSTED_OPEN_SOURCE,
+ ],
Review Comment:
<!-- Bito Reply -->
The `/resolve` command is not supported on inline comments. Please use it in
a top-level comment instead.
##########
superset/db_engine_specs/elasticsearch.py:
##########
@@ -324,6 +324,27 @@ class OpenDistroEngineSpec(BaseEngineSpec): # pylint:
disable=abstract-method
engine = "odelasticsearch"
engine_name = "OpenSearch (OpenDistro)"
+ metadata = {
+ "description": (
+ "OpenSearch (OpenDistro) SQL connector for querying OpenSearch and
"
+ "OpenDistro clusters using SQL syntax."
+ ),
+ "logo": "elasticsearch.png",
+ "homepage_url": "https://opensearch.org/";,
+ "categories": [
+ DatabaseCategory.SEARCH_NOSQL,
+ DatabaseCategory.OPEN_SOURCE,
+ ],
+ "pypi_packages": ["elasticsearch-dbapi"],
+ "connection_string":
"odelasticsearch+https://{user}:{password}@{host}:9200/";,
+ "default_port": 9200,
+ "parameters": {
+ "user": "OpenSearch username",
+ "password": "OpenSearch password",
+ "host": "OpenSearch host",
+ },
+ }
Review Comment:
<!-- Bito Reply -->
The reviewer's suggestion to extract a shared constant for the connection
string and reuse the metadata block is technically sound for reducing
duplication. However, as you noted, the `metadata` dictionary in
`lint_metadata.py` is evaluated statically via `ast` in pre-commit, which
restricts the use of shared constants or dynamic references. Given this
constraint, keeping the definitions literal is the correct approach to ensure
compatibility with the static analysis tool.
##########
superset/db_engine_specs/aurora.py:
##########
@@ -36,6 +37,27 @@ class AuroraMySQLDataAPI(MySQLEngineSpec):
"region_name={region_name}"
)
+ metadata = {
+ "description": (
+ "Amazon Aurora MySQL accessed via the AWS Data API, allowing "
+ "database queries over HTTP without managing persistent
connections."
+ ),
+ "logo": "aws-aurora.jpg",
+ "homepage_url": "https://aws.amazon.com/rds/aurora/";,
+ "categories": [
+ DatabaseCategory.CLOUD_AWS,
+ DatabaseCategory.TRADITIONAL_RDBMS,
+ DatabaseCategory.HOSTED_OPEN_SOURCE,
+ ],
+ "pypi_packages": ["sqlalchemy-aurora-data-api"],
+ "connection_string": (
+ "mysql+auroradataapi://{aws_access_id}:{aws_secret_access_key}@/"
+ "{database_name}?aurora_cluster_arn={aurora_cluster_arn}&"
+ "secret_arn={secret_arn}®ion_name={region_name}"
+ ),
Review Comment:
<!-- Bito Reply -->
The reviewer's suggestion to derive the `connection_string` from a shared
constant is technically sound for reducing duplication. However, as noted, the
static evaluation of `metadata` via `ast` in `lint_metadata.py` prevents the
use of dynamic references or shared constants in this context. Given this
constraint, repeating the template is the necessary approach to satisfy the
pre-commit checks.
##########
superset/db_engine_specs/kusto.py:
##########
@@ -223,6 +223,34 @@ class KustoKqlEngineSpec(BaseEngineSpec): # pylint:
disable=abstract-method
allows_sql_comments = False
run_multiple_statements_as_one = True
+ metadata = {
+ "description": (
+ "Azure Data Explorer (Kusto) using native Kusto Query Language
(KQL) "
+ "for high-performance log and telemetry 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"],
+ "connection_string": (
+ "kustokql+https://{cluster}.kusto.windows.net/{database}";
+ "?msi=False&azure_ad_client_id={client_id}"
+ "&azure_ad_client_secret={client_secret}"
+ "&azure_ad_tenant_id={tenant_id}"
+ ),
+ "parameters": {
+ "cluster": "Azure Data Explorer cluster name",
+ "database": "Database name",
+ "client_id": "Azure AD application (client) ID",
+ "client_secret": "Azure AD application secret",
+ "tenant_id": "Azure AD tenant ID",
+ },
+ }
Review Comment:
<!-- Bito Reply -->
Understood. Since the `metadata` block must remain a literal dictionary for
static analysis by `ast` in pre-commit, maintaining it as a literal definition
is the correct approach to satisfy the linting requirements.
--
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]
