bito-code-review[bot] commented on code in PR #41179: URL: https://github.com/apache/superset/pull/41179#discussion_r3432481490
########## tests/integration_tests/reports/commands_tests.py: ########## @@ -37,6 +36,13 @@ ) from sqlalchemy.sql import func +try: + # Flask-SQLAlchemy 3.x (required by SQLAlchemy 2.0) + from flask_sqlalchemy.query import Query as BaseQuery +except ImportError: # pragma: no cover + # Flask-SQLAlchemy 2.x + from flask_sqlalchemy import BaseQuery Review Comment: <div> <div id="suggestion"> <div id="issue"><b>Import pattern duplicated</b></div> <div id="fix"> The try/except import pattern (lines 39-44) is duplicated verbatim from `superset/queries/filters.py` (lines 19-24) and `superset/queries/saved_queries/filters.py` (lines 26-29). If the import strategy needs updating for Flask-SQLAlchemy version changes, all three locations must be modified, creating maintenance risk. </div> </div> <small><i>Code Review Run #368617</i></small> </div> --- Should Bito avoid suggestions like this for future reviews? (<a href=https://alpha.bito.ai/home/ai-agents/review-rules>Manage Rules</a>) - [ ] Yes, avoid them ########## superset/connectors/sqla/models.py: ########## @@ -335,7 +335,7 @@ def is_virtual(self) -> bool: return self.kind == DatasourceKind.VIRTUAL @declared_attr - def slices(self) -> RelationshipProperty: + def slices(self) -> Mapped[list["Slice"]]: Review Comment: <div> <div id="suggestion"> <div id="issue"><b>Incorrect type annotation for declared_attr</b></div> <div id="fix"> The return type `Mapped[list["Slice"]]` is incorrect for a `@declared_attr` method in SQLAlchemy 1.4. The `Mapped[...]` annotation style applies to direct class attribute assignments (e.g., `columns: Mapped[list[TableColumn]] = relationship(...)` at line 1304), not to `@declared_attr` methods. For `@declared_attr` methods that return ORM constructs, the return type should be `RelationshipProperty`, as confirmed by the existing pattern at line 1117 (`def datasource(self) -> RelationshipProperty:`). This mismatch can cause mypy failures and incorrect type inference. </div> <details> <summary> <b>Code suggestion</b> </summary> <blockquote>Check the AI-generated fix before applying</blockquote> <div id="code"> ``` --- a/superset/connectors/sqla/models.py +++ b/superset/connectors/sqla/models.py @@ -335,7 +335,7 @@ class BaseDatasource(Model, AuditMixinNullable, CertificationMixin, return self.kind == DatasourceKind.VIRTUAL @declared_attr - def slices(self) -> Mapped[list["Slice"]]: + def slices(self) -> RelationshipProperty: return relationship( "Slice", overlaps="table", ``` </div> </details> </div> <small><i>Code Review Run #368617</i></small> </div> --- Should Bito avoid suggestions like this for future reviews? (<a href=https://alpha.bito.ai/home/ai-agents/review-rules>Manage Rules</a>) - [ ] Yes, avoid them -- 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]
