Abdulrehman-PIAIC80387 opened a new pull request, #43239:
URL: https://github.com/apache/superset/pull/43239
### SUMMARY
Partial fix for #42980. Adds a `metadata = {...}` block on `OdpsEngineSpec`
(ODPS / Alibaba MaxCompute), the ONE spec in @jethac's 12-item incomplete list
where metadata is genuinely missing rather than deliberately consolidated in
another class.
### WHY THIS IS A NARROW FIX
Verified against `superset/db_engine_specs/lint_metadata.py --json` on
`master`: 12 specs still report `has_metadata: false`. Reading each, **11 of
the 12 explicitly point their documentation to another class in the same driver
family**:
| Spec | Docs live in |
|---|---|
| `ibmi::IBMiEngineSpec` | `db2.py`'s
`Db2EngineSpec.metadata["compatible_databases"]` |
| `kusto::KustoKqlEngineSpec` | `kusto.py`'s
`KustoSqlEngineSpec.metadata["drivers"]` |
| `elasticsearch::OpenDistroEngineSpec` | Docstring: *"consolidated in
ElasticSearchEngineSpec"* |
| 3× `databricks` legacy specs | Comment: *"Primary metadata is in
DatabricksPythonConnectorEngineSpec"* |
| 2× `aurora` DataAPI + 2× base variants | Docstring: *"in
MySQLEngineSpec/PostgresEngineSpec's compatible_databases"* |
| `clickhouse::ClickHouseEngineSpec` (sqlalchemy) | Sibling
`ClickHouseConnectEngineSpec` in same file |
Adding metadata to any of those 11 would **duplicate** existing info and
break the single-source-of-truth invariant that pattern deliberately maintains.
**`OdpsEngineSpec` is the only leaf where no parent-consolidation exists** —
users picking "ODPS (MaxCompute)" from Superset's database picker currently see
zero description or docs. That's the case this PR fixes.
### VERIFICATION
- `description` — factual from Alibaba Cloud's public product page
- `categories` — three existing `DatabaseCategory` constants
(`CLOUD_DATA_WAREHOUSES`, `ANALYTICAL_DATABASES`, `PROPRIETARY`) verified in
`base.py:217, 222, 231`
- `pypi_packages: ["pyodps"]` matches the runtime warning at
`superset/daos/database.py:279`
- `connection_string` matches the ODPS URI regex at
`superset/daos/database.py:284` and the test fixtures in
`tests/unit_tests/db_engine_specs/test_odps.py:102`
- `logo` / `default_port` intentionally omitted — no aliyun/maxcompute asset
in `docs/static/img/databases/` (78 files, none Alibaba-related) and MaxCompute
uses a URL endpoint rather than a fixed host:port. Both are `RECOMMENDED`, not
`REQUIRED` — linter still passes.
### RESULT (verified locally)
Before:
```
{"total": 80, "with_metadata": 68, "all_required": 68, "average_score": 73.7}
```
After:
```
{"total": 80, "with_metadata": 69, "all_required": 69, "average_score": 74.6}
```
ODPS report after change: `has_metadata: true`, `missing_required: []`,
`completeness_score: 71.8`.
### DEFENSIVE TEST
Small pytest in `tests/unit_tests/db_engine_specs/test_odps.py` pins the
four `REQUIRED_FIELDS` + the `pyodps` package name so an accidental future
removal of the metadata block trips a fast pytest failure, not just the
standalone linter script (which isn't part of the CI test surface).
### IF YOU'D RATHER CLEAR ALL 12
Three follow-up options ready to open as a separate PR:
1. **F1 (recommended):** teach `lint_metadata.py` to detect the
*"Documentation ... consolidated in X"* docstring pattern and mark those leaves
as `consolidated` rather than `false`. Answers @jethac's own design question
directly; ~40 LOC in one file; no metadata duplication.
2. **F2:** add a machine-readable `consolidated_metadata_in = "..."` class
attribute on each consolidated leaf. 11 files touched (small change each); zero
regex.
3. **F3 (last resort):** duplicate the metadata on every leaf. 11 files,
~150-250 LOC total, breaks single-source-of-truth. Not recommended — every
future metadata change now has to be applied in multiple places.
Happy to open the follow-up in whichever shape you prefer. This PR ships the
narrow ODPS fix first so at least the one genuine gap is closed.
### ADDITIONAL INFORMATION
- [x] Has associated issue: partial fix for #42980
- [x] Required feature flags: none
- [x] Changes UI: no (metadata surfaces in the DB picker; existing UI code
just reads it)
- [x] Includes DB Migration: no
- [x] Includes CLI or Node.js commands: no
- [x] Breaking change: no (purely additive)
--
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]
