You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by "Abdulrehman-PIAIC80387 (via GitHub)" <gi...@apache.org> on 2026/08/17 06:59:13 UTC
[PR] feat(db_engine_specs): add metadata block to OdpsEngineSpec [superset]
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat(db_engine_specs): add metadata block to OdpsEngineSpec [superset]
Posted by "rusackas (via GitHub)" <gi...@apache.org>.
rusackas merged PR #43239:
URL: https://github.com/apache/superset/pull/43239
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat(db_engine_specs): add metadata block to OdpsEngineSpec [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #43239:
URL: https://github.com/apache/superset/pull/43239#issuecomment-5312858191
AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat(db_engine_specs): add metadata block to OdpsEngineSpec [superset]
Posted by "codecov[bot] (via GitHub)" <gi...@apache.org>.
codecov[bot] commented on PR #43239:
URL: https://github.com/apache/superset/pull/43239#issuecomment-5312905310
## [Codecov](https://app.codecov.io/gh/apache/superset/pull/43239?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
:white_check_mark: All modified and coverable lines are covered by tests.
:white_check_mark: Project coverage is 57.27%. Comparing base ([`d114eb6`](https://app.codecov.io/gh/apache/superset/commit/d114eb638b7a10b5e3c7eea78a43ed0019639e7a?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`0144a91`](https://app.codecov.io/gh/apache/superset/commit/0144a91c80ee51fea9519f011326545656186533?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## master #43239 +/- ##
==========================================
- Coverage 66.66% 57.27% -9.39%
==========================================
Files 2873 2873
Lines 163654 163655 +1
Branches 37771 37771
==========================================
- Hits 109093 93730 -15363
- Misses 52432 69036 +16604
+ Partials 2129 889 -1240
```
| [Flag](https://app.codecov.io/gh/apache/superset/pull/43239/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
|---|---|---|
| [hive](https://app.codecov.io/gh/apache/superset/pull/43239/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `38.17% <100.00%> (+<0.01%)` | :arrow_up: |
| [mysql](https://app.codecov.io/gh/apache/superset/pull/43239/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [postgres](https://app.codecov.io/gh/apache/superset/pull/43239/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [presto](https://app.codecov.io/gh/apache/superset/pull/43239/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `40.11% <100.00%> (+<0.01%)` | :arrow_up: |
| [python](https://app.codecov.io/gh/apache/superset/pull/43239/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `40.17% <100.00%> (-19.10%)` | :arrow_down: |
| [sqlite](https://app.codecov.io/gh/apache/superset/pull/43239/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [unit](https://app.codecov.io/gh/apache/superset/pull/43239/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#carryforward-flags-in-the-pull-request-comment) to find out more.
</details>
[:umbrella: View full report in Codecov by Harness](https://app.codecov.io/gh/apache/superset/pull/43239?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
<details><summary> :rocket: New features to boost your workflow: </summary>
- :snowflake: [Test Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests, report on failures, and find test suite problems.
- :package: [JS Bundle Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
</details>
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org