rusackas commented on code in PR #44817: URL: https://github.com/apache/superset/pull/44817#discussion_r4211418778
########## superset/models/helpers.py: ########## @@ -738,9 +738,20 @@ def convert_uuids(obj: Any) -> Any: class UUIDMixin: # pylint: disable=too-few-public-methods - uuid = sa.Column( - UUIDType(binary=True), primary_key=False, unique=True, default=uuid.uuid4 - ) + # A plain `uuid = sa.Column(...)` class attribute resolves to `Any` (or + # the raw `Column[Any]` descriptor) under mypy unless the whole class + # hierarchy happens to be loaded together in the same run -- so a + # scoped, per-PR mypy check silently lets an unnarrowed `.uuid` read + # through even though the column is nullable, and only a full-repo + # sweep catches the mismatch later (see #44789 and its predecessors + # #44394/#44424/#44648). The `@declared_attr` + explicit `Mapped[...]` + # return annotation used by `created_by_fk`/`changed_by_fk` above makes + # the type deterministic regardless of what else mypy is checking. + @declared_attr Review Comment: Good call, that works. Pushed `tests/mypy_fixtures/uuid_mixin_nullable.py` (an `assert_type` on `Dashboard.uuid`) plus a manual-stage `mypy (sole-input fixtures)` hook that CI runs with that one file as input, `--no-incremental` so it can't lean on the main hook's cache. Confirmed it goes red with the declaration reverted (`Expression is of type "Column[Any]", not "UUID | None"`) and clean with it in place. -- 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]
