kaxil commented on code in PR #63491:
URL: https://github.com/apache/airflow/pull/63491#discussion_r3439860865
##########
airflow-core/src/airflow/executors/base_executor.py:
##########
@@ -471,6 +490,22 @@ def trigger_tasks(self, open_slots: int) -> None:
if workload_list:
self._process_workloads(workload_list)
+ def trigger_tasks(self, open_slots: int) -> None:
+ """Backward-compat shim: forwards to :meth:`trigger_workloads`."""
+ self._warn_legacy_property(
+ "trigger_tasks",
+ "trigger_tasks is deprecated, use trigger_workloads instead.",
+ )
+ self.trigger_workloads(open_slots)
+
+ def order_queued_tasks_by_priority(self) -> list:
+ """Backward-compat shim: forwards to
:meth:`_get_workloads_to_schedule`."""
+ self._warn_legacy_property(
+ "order_queued_tasks_by_priority",
+ "order_queued_tasks_by_priority is deprecated, use
_get_workloads_to_schedule instead.",
+ )
+ return self._get_workloads_to_schedule(self.parallelism -
len(self.running))
Review Comment:
Thanks for adding the shim. One fidelity gap worth a docstring note: the old
`order_queued_tasks_by_priority` returned all queued tasks, tasks-only and
untruncated (`sorted(self.queued_tasks.items(), ...)`). This shim forwards to
`_get_workloads_to_schedule(self.parallelism - len(self.running))`, which
iterates every queue in `executor_queues` (so callbacks and connection-tests
are included) and truncates to the open-slot count. A downstream subclass that
called this to inspect its full task queue would now get a shorter,
callback-polluted list. Minor since it's deprecated, but the changed return
shape is worth calling out.
##########
providers/amazon/src/airflow/providers/amazon/aws/executors/ecs/ecs_executor.py:
##########
@@ -169,15 +155,15 @@ def _process_workloads(self, workload_items:
Sequence[workloads.All]) -> None:
queue = workload.ti.queue
executor_config = workload.ti.executor_config or {}
- del self.queued_tasks[key]
+ del self.executor_queues[WorkloadType.EXECUTE_TASK][key]
Review Comment:
Unlike the Lambda, Batch, and K8s executors -- which gate this exact line
with `if AIRFLOW_V_3_3_PLUS: del
self.executor_queues[WorkloadType.EXECUTE_TASK][key]` / `else: del
self.queued_tasks[key]` -- the ECS `ExecuteTask` branch deletes from
`self.executor_queues[WorkloadType.EXECUTE_TASK]` unconditionally.
The amazon provider supports `apache-airflow>=2.11.0`, but `executor_queues`
only exists on the 3.3+ base, and `WorkloadType` is imported only under the
`AIRFLOW_V_3_3_PLUS` guard at the top of this file. So on Airflow 3.0-3.2 every
ECS task dispatch hits this line and raises `AttributeError`/`NameError`. The
callback branch just below is correctly gated (`elif AIRFLOW_V_3_3_PLUS and
isinstance(...)`) -- the task branch needs the same `else: del
self.queued_tasks[key]` fallback that Lambda (L239-242), Batch (L140-143), and
K8s (L247-250) already have.
--
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]
