shahar1 commented on code in PR #74629:
URL: https://github.com/apache/airflow/pull/74629#discussion_r4306214255
##########
airflow-core/src/airflow/models/task_coordinates.py:
##########
@@ -354,14 +354,16 @@ def prefetch_regional_tasks(self, dag_id: str, run_id:
str, task_ids: Collection
for task_id in task_ids:
self._regional_tasks[dag_id, run_id, task_id] = task_id in regional
+ # TODO: annotate the return type as Select[UUID] once the minimum
SQLAlchemy is 2.1;
Review Comment:
**nit** — The same two-line TODO appears 8 times across `dynamic_region.py`,
`task_coordinates.py` and `xcom.py`. AGENTS.md says:
> Do not repeat comments at each call site.
You could define one alias with a single TODO, for example `UUIDSelect:
TypeAlias = Select` (and one for `XComModel`), and use it in these signatures.
The 2.1 migration then becomes a one-line change and is easy to grep for. Not
blocking.
##########
airflow-core/src/airflow/api_fastapi/auth/managers/base_auth_manager.py:
##########
@@ -786,7 +786,8 @@ def get_authorized_pools(
rows = session.execute(stmt).all()
pools_by_team: dict[str | None, set[str]] = defaultdict(set)
for pool_name, team_name in rows:
- pools_by_team[team_name].add(pool_name)
+ if pool_name is not None:
Review Comment:
**minor** — This is the only change here that alters runtime behaviour: the
`select(Pool.pool, Pool.team_name)` above has no `WHERE`, and `pool` is
`nullable=True`. So a NULL-named row used to reach `filter_authorized_pools`
and now gets dropped. AGENTS.md says:
> Every changed or added behaviour must have a test; every test must fail
without the PR's change.
The PR body already notes the test is missing. A small test with one
`Pool(pool=None)` row, asserting that `None` is not in the result, would cover
it.
--
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]
