Dev-iL commented on code in PR #70793:
URL: https://github.com/apache/airflow/pull/70793#discussion_r3694601532
##########
airflow-core/src/airflow/utils/db.py:
##########
@@ -1104,11 +1104,20 @@ def reflect_tables(tables: list[MappedClassProtocol |
str] | None, session, sche
else:
for tbl in tables:
try:
- table_name = tbl if isinstance(tbl, str) else tbl.__tablename__
tbl_schema: str | None
- tbl_schema, sep, name = table_name.partition(".")
- if not sep:
- tbl_schema, name = None, table_name
+ if isinstance(tbl, str):
+ tbl_schema, sep, name = tbl.partition(".")
+ if not sep:
+ tbl_schema, name = None, tbl
+ else:
+ # A mapped class already carries its configured schema
(e.g. via
+ # ``sql_alchemy_schema``) on its table; a bare
``__tablename__`` would
+ # discard it and fall back to the connection's default
schema.
+ # ``__table__`` isn't declared on ``MappedClassProtocol``
(SQLAlchemy sets
+ # it dynamically, invisible to mypy's static checks here),
so read it via
+ # ``getattr`` instead of a direct attribute access.
+ name = tbl.__tablename__
+ tbl_schema = getattr(tbl, "__table__").schema
Review Comment:
Consider using the `local_table` property available via [SQLA's inspect
api](https://docs.sqlalchemy.org/en/20/orm/declarative_tables.html#accessing-table-and-metadata)
instead of `getattr`. I am not 100% sure about this, but prrhaps it could
satisfy mypy, so type safety is retained.
##########
airflow-core/src/airflow/utils/db.py:
##########
@@ -1104,11 +1104,20 @@ def reflect_tables(tables: list[MappedClassProtocol |
str] | None, session, sche
else:
for tbl in tables:
try:
- table_name = tbl if isinstance(tbl, str) else tbl.__tablename__
tbl_schema: str | None
- tbl_schema, sep, name = table_name.partition(".")
- if not sep:
- tbl_schema, name = None, table_name
+ if isinstance(tbl, str):
+ tbl_schema, sep, name = tbl.partition(".")
+ if not sep:
+ tbl_schema, name = None, tbl
+ else:
+ # A mapped class already carries its configured schema
(e.g. via
Review Comment:
I would trim the entire comment down to a few words or even remove it
compeletely and let users refer to the PR for more context.
--
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]
