Skip to content

Commit 5ea30a3

Browse files
antonio-mello-aiclaude
authored andcommitted
[v3-2-test] Fix TypeError in GET /dags/{dag_id}/tasks when order_by field has None values (apache#64384)
The tasks endpoint crashed with a 500 Internal Server Error when sorting by a field (e.g. start_date) that contains None values, because Python 3 cannot compare None with None using '<'. This adds explicit validation of the order_by parameter against a whitelist of sortable fields (returning 400 for invalid fields, consistent with SortParam used in other endpoints) and handles None values in the sort key so nullable fields work correctly. Closes: apache#63927 (cherry picked from commit 15cf396) Co-authored-by: Antonio Mello <ajgcvm@gmail.com> Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent 51144dc commit 5ea30a3

2 files changed

Lines changed: 49 additions & 7 deletions

File tree

  • airflow-core
    • src/airflow/api_fastapi/core_api/routes/public
    • tests/unit/api_fastapi/core_api/routes/public

airflow-core/src/airflow/api_fastapi/core_api/routes/public/tasks.py

Lines changed: 35 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,6 @@
1717

1818
from __future__ import annotations
1919

20-
from operator import attrgetter
2120
from typing import cast
2221

2322
from fastapi import Depends, HTTPException, status
@@ -33,6 +32,29 @@
3332

3433
tasks_router = AirflowRouter(tags=["Task"], prefix="/dags/{dag_id}/tasks")
3534

35+
_SORTABLE_TASK_FIELDS = {
36+
"task_id",
37+
"task_display_name",
38+
"owner",
39+
"start_date",
40+
"end_date",
41+
"trigger_rule",
42+
"depends_on_past",
43+
"wait_for_downstream",
44+
"retries",
45+
"queue",
46+
"pool",
47+
"pool_slots",
48+
"execution_timeout",
49+
"retry_delay",
50+
"retry_exponential_backoff",
51+
"priority_weight",
52+
"weight_rule",
53+
"ui_color",
54+
"ui_fgcolor",
55+
"operator_name",
56+
}
57+
3658

3759
@tasks_router.get(
3860
"",
@@ -52,10 +74,18 @@ def get_tasks(
5274
) -> TaskCollectionResponse:
5375
"""Get tasks for DAG."""
5476
dag = get_latest_version_of_dag(dag_bag, dag_id, session)
55-
try:
56-
tasks = sorted(dag.tasks, key=attrgetter(order_by.lstrip("-")), reverse=(order_by[0:1] == "-"))
57-
except AttributeError as err:
58-
raise HTTPException(status.HTTP_400_BAD_REQUEST, str(err))
77+
lstripped_order_by = order_by.lstrip("-")
78+
if lstripped_order_by not in _SORTABLE_TASK_FIELDS:
79+
raise HTTPException(
80+
status.HTTP_400_BAD_REQUEST,
81+
f"Ordering with '{lstripped_order_by}' is disallowed or "
82+
f"the attribute does not exist on the model",
83+
)
84+
tasks = sorted(
85+
dag.tasks,
86+
key=lambda task: (getattr(task, lstripped_order_by) is None, getattr(task, lstripped_order_by)),
87+
reverse=(order_by[0:1] == "-"),
88+
)
5989
return TaskCollectionResponse(
6090
tasks=cast("list[TaskResponse]", tasks),
6191
total_entries=len(tasks),

airflow-core/tests/unit/api_fastapi/core_api/routes/public/test_tasks.py

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -543,10 +543,22 @@ def test_should_raise_400_for_invalid_order_by_name(self, test_client):
543543
f"{self.api_prefix}/{self.dag_id}/tasks?order_by=invalid_task_colume_name",
544544
)
545545
assert response.status_code == 400
546-
assert (
547-
response.json()["detail"] == "'EmptyOperator' object has no attribute 'invalid_task_colume_name'"
546+
assert response.json()["detail"] == (
547+
"Ordering with 'invalid_task_colume_name' is disallowed or "
548+
"the attribute does not exist on the model"
548549
)
549550

551+
def test_should_respond_200_order_by_start_date_with_none(self, test_client):
552+
"""Sorting by a nullable field should not raise TypeError (issue #63927)."""
553+
response = test_client.get(
554+
f"{self.api_prefix}/{self.unscheduled_dag_id}/tasks?order_by=start_date",
555+
)
556+
assert response.status_code == 200
557+
tasks = response.json()["tasks"]
558+
assert len(tasks) == 2
559+
# All start_dates are None for unscheduled tasks; verify they sort without error
560+
assert all(t["start_date"] is None for t in tasks)
561+
550562
def test_should_respond_404(self, test_client):
551563
dag_id = "xxxx_not_existing"
552564
response = test_client.get(f"{self.api_prefix}/{dag_id}/tasks")

0 commit comments

Comments
 (0)