mikebridge opened a new pull request, #43886:
URL: https://github.com/apache/superset/pull/43886
### SUMMARY
Fixes SC-119510: the semantic-layer query mapper duck-typed undeclared
`.metrics` / `.dimensions` **attributes** on the provider view
(`query_object.datasource.implementation`), but the superset-core
`SemanticView` ABC declares only the abstract **methods** `get_metrics()` /
`get_dimensions()`. A provider built strictly to the documented ABC therefore
satisfies the interface yet crashes at query time — the first `AttributeError`
fires in `_validate_metrics` and surfaces as an HTTP 500 `{"message": "Fatal
error"}` on the first chart query. Explore and metadata endpoints call the
methods, so a strictly-ABC provider *looks* healthy until a chart queries it.
The shipped Snowflake/Cube/MetricFlow extensions dodge the bug only by
convention (they happen to define the attributes).
The fix removes the dual contract outright, without touching the
superset-core ABC: all 11 duck-typed sites across the 6 mapper functions
(`map_query_object`, `_validate_metrics`, `_validate_dimensions`,
`_validate_granularity`, `_validate_group_limit`, `_validate_orderby`) now call
the abstract methods the mapper already requires, fetching once per function
into a local. This also matches the existing convention elsewhere
(`superset/semantic_layers/models.py` already calls
`implementation.get_metrics()`).
Test strategy makes the whole suite the regression proof:
- `MockSemanticView` now stores its sets privately and exposes **only** the
ABC surface — with the fix reverted, **34 mapper tests fail** with the exact
defect signature.
- A new `AbcOnlyView` fixture subclasses the real `SemanticView` ABC
(methods only, name-mangled storage) and pins the end-to-end validate+map path
that used to 500 (`test_abc_only_provider_validates_and_maps`).
- Five tests that hand-rolled `mocker.Mock()` implementations with attribute
assignment now build real `MockSemanticView` instances.
### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
N/A — backend contract fix. Before: `AttributeError: 'AbcOnlyView' object
has no attribute 'metrics'` → HTTP 500 on `POST /api/v1/chart/data`. After: the
query maps and executes.
### TESTING INSTRUCTIONS
```bash
python -m pytest tests/unit_tests/semantic_layers -q # 437 passed
```
Reverted-fix control: `git stash push -- superset/ && python -m pytest
tests/unit_tests/semantic_layers/mapper_test.py -q` → 34 failed; `git stash
pop`.
### ADDITIONAL INFORMATION
- [x] Has associated issue: SC-119510 (found during SC-106828 / #42760
Phase-1 validation)
- [ ] Required feature flags:
- [ ] Changes UI
- [ ] Includes DB Migration (follow approval process in
[SIP-59](https://github.com/apache/superset/issues/13351))
- [ ] Introduces new feature or API
- [ ] Removes existing feature or API
🤖 Generated with [Claude Code](https://claude.com/claude-code)
https://claude.ai/code/session_01JRLEJS4mUqKBoPjSjviKUW
--
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]
