sadpandajoe commented on code in PR #44817: URL: https://github.com/apache/superset/pull/44817#discussion_r4152928937
########## 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: A regression in scoped UUID inference would still pass the existing runtime tests, which do not check the nullable type this fixes. Could we check in a standalone mypy fixture that asserts `Dashboard.uuid` is `UUID | None` with only that fixture as input, and verify it fails when this declaration is reverted? -- 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]
