yousoph commented on code in PR #42404:
URL: https://github.com/apache/superset/pull/42404#discussion_r3833346689
##########
superset/models/core.py:
##########
@@ -144,6 +144,12 @@ class Theme(AuditMixinNullable, ImportExportMixin, Model):
is_system_default = Column(Boolean, default=False, nullable=False)
is_system_dark = Column(Boolean, default=False, nullable=False)
+ editors = relationship(
+ "Subject",
+ secondary="theme_editors",
+ passive_deletes=True,
+ )
Review Comment:
This is a false positive. The relationship uses a **string** target —
`relationship("Subject", secondary="theme_editors", ...)` — which SQLAlchemy
resolves via its declarative class registry at mapper-configuration time, not
via Python name resolution, so no import of `Subject` in `core.py` is required
and there's no NameError. `dashboard.py` uses the bare `Subject` class (hence
its import); `Theme` intentionally uses the string form to avoid a circular
import in the models-package init chain. If it were an undefined name,
`core.py` would fail at import and all backend tests would fail — they pass. No
change needed.
--
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]
