fix mypy in taskmap.py #57862
fix mypy in taskmap.py #57862
Conversation
|
|
||
| USE_ROW_LEVEL_LOCKING: bool = conf.getboolean("scheduler", "use_row_level_locking", fallback=True) | ||
|
|
||
| T = TypeVar("T", bound=tuple[Any, ...]) |
There was a problem hiding this comment.
Is there a benefit to tuple[Any, ...] over a simple tuple?
There was a problem hiding this comment.
Hi @Dev-iL, good point. I used tuple[Any, ...] to be more explicit.
It clearly shows it’s a variable-length tuple (...) of any type (Any),
which I find is a bit more precise and readable in the modern syntax.
henry3260
force-pushed
the
fix-mypy-taskmap
branch
3 times, most recently
from
7bfbe0c to
77197e2
Compare
This comment has been minimized.
This comment has been minimized.
henry3260
force-pushed
the
fix-mypy-taskmap
branch
3 times, most recently
from
1f50bf8 to
954ad86
Compare
|
Hi @potiuk , it’s ready for merge now. Thanks for reviewing!❤️ |
henry3260
force-pushed
the
fix-mypy-taskmap
branch
from
954ad86 to
8ae1f01
Compare
| :meta private: | ||
| """ | ||
| locked_rows = with_row_locks(query, session) | ||
| select_stmt = cast("Select", query.statement) |
There was a problem hiding this comment.
Hmm, why do we need the cast here? (And it’s a bit weird since the query argument is already a Select?)
There was a problem hiding this comment.
I believe the cast will become obsolete by #55274
There was a problem hiding this comment.
Yes, Once #55274 is merged, only Select will remain there—and your mypy will be happy.
There was a problem hiding this comment.
Thanks for the update! I will wait for your PR to be merged and proceed after that.
| locked_rows = with_row_locks(query, session) | ||
| select_stmt = cast("Select", query.statement) | ||
| locked_statement = with_row_locks(select_stmt, session) | ||
| locked_rows = session.execute(locked_statement).all() |
There was a problem hiding this comment.
Do we have to call all() here? It’s quite inefficient especially since we don’t actually use the value.
|
Closing this one because this is being fixed in #58652 |
5 participants
Sub task of #56735.
Part of this issue
fixed errors:
^ Add meaningful description above
Read the Pull Request Guidelines for more information.
In case of fundamental code changes, an Airflow Improvement Proposal (AIP) is needed.
In case of a new dependency, check compliance with the ASF 3rd Party License Policy.
In case of backwards incompatible changes please leave a note in a newsfragment file, named
{pr_number}.significant.rstor{issue_number}.significant.rst, in airflow-core/newsfragments.