sadpandajoe opened a new pull request, #44610:
URL: https://github.com/apache/superset/pull/44610
### SUMMARY
The nightly full-repo lint (`prek run --all-files`, run 35964300811 on
8141d666d6) reports 6 mypy errors that per-PR CI never saw:
- `superset/mcp_service/dataset_scope.py:127` — two `union-attr` errors on
`SqlaTable.uuid.in_()`, introduced by #44146.
- `tests/unit_tests/mcp_service/dataset/tool/test_dataset_tools.py:2712` —
`attr-defined` on `SqlaTable.id.in_()`, same PR.
- `tests/unit_tests/charts/semantic_view_chart_filter_test.py:402-404` —
three `assignment` errors putting `perm`/`schema_perm`/`catalog_perm` into
`str`-annotated locals, introduced by #43848.
Root cause: `superset_core.common.models.Dataset` declares `id: int`,
`uuid: UUID | None` and `perm/schema_perm/catalog_perm: str | None` as plain
value annotations. `SqlaTable` lists `CoreDataset` first in its bases, so
those annotations shadow the `Column(...)` assignments on `BaseDatasource`
and `UUIDMixin`, and mypy stops seeing query-buildable column expressions.
The values are real, queryable columns at runtime — this is a static-only
artifact of how superset-core documents the ORM surface.
The fix keeps the reported call sites working and matches what the repo
already does for the identical pattern at
`superset/connectors/sqla/models.py:2491`: a targeted `type: ignore` on the
two `.in_()` call sites, and the existing assert-not-None narrowing idiom for
the three nullable reads in the chart-filter test.
It also adds the other half of that precedent. `warn_unused_ignores` is on
globally, and the annotations above only resolve when superset-core's sources
are part of the same mypy run. Under `prek run --files <changed>` —
per-PR CI, and a local `pre-commit run` — `superset_core` is unresolvable and
falls back to `Any`, so the `.in_()` calls check clean and both new ignores
would be flagged `unused-ignore`. The two modules are therefore exempted from
`warn_unused_ignores`, the same remedy already applied to
`superset.connectors.sqla.models` and friends.
### TESTING INSTRUCTIONS
1. Full-repo lint, which is what the nightly runs:
`pre-commit run mypy --all-files` — clean on this branch, 6 errors
without it.
2. Changed-files lint, which is what per-PR CI runs:
`git add -A && pre-commit run mypy` — clean on this branch. Drop the
`pyproject.toml` hunk and it reports two `unused-ignore` errors instead.
3. Confirm no behavior changed:
`pytest tests/unit_tests/mcp_service/test_dataset_scope.py
tests/unit_tests/charts/semantic_view_chart_filter_test.py
tests/unit_tests/mcp_service/dataset/tool/test_dataset_tools.py`
### ADDITIONAL INFORMATION
- [x] Has associated issue: Fixes #44609
- [ ] Required feature flags:
- [ ] Changes UI
- [ ] Includes DB Migration (follow approval process in SIP-59)
- [ ] Migration is atomic, supports rollback & is backwards-compatible
- [ ] Confirm DB migration upgrade and downgrade tested
- [ ] Runtime estimates and downtime expectations provided
- [ ] Introduces new feature or API
- [ ] Removes existing feature or API
--
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]
