rusackas opened a new pull request, #44817:
URL: https://github.com/apache/superset/pull/44817
### SUMMARY
Follow-up to #44789 (and its predecessors #44394, #44424, #44648) — same
defect class, four separate fixup PRs now, so this addresses the root cause
instead of the fourth symptom.
`UUIDMixin.uuid` (`superset/models/helpers.py`) is declared as a plain mixin
class attribute: `uuid = sa.Column(UUIDType(binary=True), ...)`. Under mypy,
that resolves to `Any` (or the bare `Column[Any]` descriptor) unless the whole
class hierarchy happens to already be loaded together in the same run.
Reproduced directly: `reveal_type(some_dashboard.uuid)` in an isolated one-file
mypy check returns `Any`/`Column[Any]`, not `UUID | None` — even though the
column is nullable and every one of the four prior PRs had to fix exactly this
mismatch.
That's why this keeps recurring as a fixup PR instead of getting caught when
the offending code is written: the per-PR `pre-commit (current)` check only
lints the files changed in that PR, and in that narrow scope mypy can't resolve
`.uuid`'s real type, so an unnarrowed read passed into a callee requiring a
non-Optional `UUID` sails through silently. Only the nightly full-repo sweep
(`pre-commit.yml`'s `schedule: cron` job) has enough context to resolve the
type correctly and catch the mismatch — by which point it's already on `master`.
**Fix**: switch `UUIDMixin.uuid` to the `@declared_attr` + explicit
`Mapped[Optional[uuid.UUID]]` return-annotation pattern that
`created_by_fk`/`changed_by_fk` already use a few lines above it in the same
file. This makes the type resolve deterministically regardless of what else
mypy happens to be checking in that run.
No runtime behavior change — `@declared_attr` returning a `Column` is the
exact same mechanism SQLAlchemy already uses for those two FK columns.
### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
N/A (typing-only change, no UI).
### TESTING INSTRUCTIONS
Verified directly rather than just asserted:
1. **Before** (checked out #44514's pre-fix head, mypy scoped to just its
two new test files): 0 errors reported for either file, despite the exact
`RestoreDatasetVersionCommand(dataset.uuid, target)` mismatch #44789 later had
to fix. `reveal_type(dashboard.uuid)` in an isolated file: `Any` /
`Column[Any]`.
2. **After this change**, same isolated-file check:
`reveal_type(dashboard.uuid)` → `Union[uuid.UUID, None]`. A synthetic repro of
the #44789-class bug (`RestoreDashboardVersionCommand(d.uuid, d.uuid)` in a
fresh one-file mypy run) is now correctly flagged:
```
error: Argument 1 to "RestoreDashboardVersionCommand" has incompatible
type "UUID | None"; expected "UUID" [arg-type]
error: Argument 2 to "RestoreDashboardVersionCommand" has incompatible
type "UUID | None"; expected "UUID" [arg-type]
```
3. `pre-commit run --files superset/models/helpers.py` — clean (mypy, ruff,
pylint all pass; the `# pylint: disable=arguments-renamed` on the new method
mirrors the identical suppression already present on
`created_by_fk`/`changed_by_fk`).
4. `pytest tests/unit_tests/versioning/
tests/unit_tests/databases/commands/importers/v1/import_test.py
tests/unit_tests/examples/generic_loader_test.py
tests/unit_tests/themes/model_test.py tests/unit_tests/datasets/
tests/unit_tests/dashboards/` — 444 passed, no regressions across every test
path that exercises `.uuid` (import/export UUID coercion, versioning restore,
dataset/dashboard model construction).
### ADDITIONAL INFORMATION
- [ ] Has associated issue:
- [ ] Required feature flags:
- [ ] Changes UI
- [ ] Includes DB Migration (follow approval process in
[SIP-59](https://github.com/apache/superset/issues/13351))
- [ ] 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
🤖 Generated with [Claude Code](https://claude.com/claude-code)
--
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]
