rusackas commented on code in PR #44817: URL: https://github.com/apache/superset/pull/44817#discussion_r4154811007
########## 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: I actually tried this: a standalone fixture importing just `UUIDMixin` took mypy well over two minutes to resolve the import graph on its own, so it is not really viable as a routine pytest check. The `reveal_type` check in the PR description already demonstrates the regression class by hand. Open to a cheaper way to pin it down if you have one. -- 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]
