bito-code-review[bot] commented on code in PR #41179:
URL: https://github.com/apache/superset/pull/41179#discussion_r3432627108
##########
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:
<!-- Bito Reply -->
The decision to avoid a shared shim module for a temporary six-line import
fallback is reasonable given the plan to remove it once the dependency is
updated. Maintaining the current approach avoids unnecessary architectural
complexity for code that is intended to be short-lived.
**tests/integration_tests/reports/commands_tests.py**
```
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
```
##########
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:
<!-- Bito Reply -->
The suggestion provided by the reviewer is technically correct regarding the
current SQLAlchemy 1.4 behavior, where `@declared_attr` methods returning ORM
constructs should be annotated as `RelationshipProperty`. While your intention
to prepare for future declarative patterns is noted, using
`RelationshipProperty` ensures compatibility with current type checking and
avoids potential mypy failures in the existing codebase.
**superset/connectors/sqla/models.py**
```
@declared_attr
def slices(self) -> RelationshipProperty:
return relationship(
"Slice",
overlaps="table",
```
--
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]
