Skip to content

Remove note from grid view response #51225

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
jedcunningham wants to merge 1 commit into from
Closed

Remove note from grid view response #51225

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion airflow-core/src/airflow/api_fastapi/core_api/datamodels/ui/grid.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,6 @@ class GridTaskInstanceSummary(BaseModel):
child_states: dict[str, int] | None
task_count: int
state: TaskInstanceState | None
note: str | None


class GridDAGRunwithTIs(BaseModel):
Expand Down
6 changes: 0 additions & 6 deletions airflow-core/src/airflow/api_fastapi/core_api/openapi/_private_ui.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1478,11 +1478,6 @@ components:
anyOf:
- $ref: '#/components/schemas/TaskInstanceState'
- type: 'null'
note:
anyOf:
- type: string
- type: 'null'
title: Note
type: object
required:
- task_id
Expand All @@ -1493,7 +1488,6 @@ components:
- child_states
- task_count
- state
- note
title: GridTaskInstanceSummary
description: Task Instance Summary model for the Grid UI.
HTTPExceptionResponse:
Expand Down
1 change: 0 additions & 1 deletion airflow-core/src/airflow/api_fastapi/core_api/routes/ui/grid.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,6 @@ def grid_data(
# Retrieve, sort and encode the Task Instances
tis_of_dag_runs, _ = paginated_select(
statement=select(TaskInstance)
Comment on lines 134 to 135
Copy link
Collaborator

@sunank200 sunank200 May 30, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
tis_of_dag_runs, _ = paginated_select(
statement=select(TaskInstance)
tis_of_dag_runs, _ = paginated_select(
statement=select(TaskInstance)
.options(selectinload(TaskInstance.task_instance_note))

Instead of dropping the note column entirely, I wonder if we can get back most of the performance win by switching from OUTER JOIN to a direct select‐in load on the note relationship. https://docs.sqlalchemy.org/en/13/orm/loading_relationships.html#sqlalchemy.orm.selectinload

Copy link
Collaborator

@sunank200 sunank200 May 30, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

May be something like this:


statement = (
    select(TaskInstance)
    .options(selectinload(TaskInstance.task_instance_note))
    .where(TaskInstance.dag_id == dag.dag_id)
    .where(TaskInstance.run_id.in_([dr.run_id for dr in dag_runs]))
)

tis_of_dag_runs, _ = paginated_select(
    statement=statement,
    filters=[],
    order_by=SortParam(allowed_attrs=["task_id", "run_id"], model=TaskInstance).set_value("task_id"),
    offset=offset,
    limit=None,
)

This will emit two smaller queries. One for all TIs, one for their notes, rather than one giant join. But we'll still have ti.note populated. Can we try this as well and see if it improves time better?

Copy link
Member Author

@jedcunningham jedcunningham May 30, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is basically as performant. I was also aiming for a smaller response, but this field is worth keeping (I have another follow up cutting a lot more out, keep an eye out for that one 😄). Thanks!

.join(TaskInstance.task_instance_note, isouter=True)
.where(TaskInstance.dag_id == dag.dag_id)
.where(TaskInstance.run_id.in_([dag_run.run_id for dag_run in dag_runs])),
filters=[],
Expand Down
2 changes: 0 additions & 2 deletions airflow-core/src/airflow/api_fastapi/core_api/services/ui/grid.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -185,7 +185,6 @@ def fill_task_instance_summaries(
ti_start_date = min([ti.start_date for ti in tis if ti.start_date], default=None)
ti_end_date = max([ti.end_date for ti in tis if ti.end_date], default=None)
ti_queued_dttm = min([ti.queued_dttm for ti in tis if ti.queued_dttm], default=None)
ti_note = min([ti.note for ti in tis if ti.note], default=None)

# Calculate the child states for the task
# Initialize the child states with 0
Expand Down Expand Up @@ -242,7 +241,6 @@ def fill_task_instance_summaries(
state=TaskInstanceState[overall_ti_state.upper()]
if overall_ti_state != "no_status"
else None,
note=ti_note,
)
)

Expand Down
12 changes: 0 additions & 12 deletions airflow-core/src/airflow/ui/openapi-gen/requests/schemas.gen.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7078,17 +7078,6 @@ export const $GridTaskInstanceSummary = {
},
],
},
note: {
anyOf: [
{
type: "string",
},
{
type: "null",
},
],
title: "Note",
},
},
type: "object",
required: [
Expand All @@ -7100,7 +7089,6 @@ export const $GridTaskInstanceSummary = {
"child_states",
"task_count",
"state",
"note",
],
title: "GridTaskInstanceSummary",
description: "Task Instance Summary model for the Grid UI.",
Expand Down
1 change: 0 additions & 1 deletion airflow-core/src/airflow/ui/openapi-gen/requests/types.gen.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1742,7 +1742,6 @@ export type GridTaskInstanceSummary = {
} | null;
task_count: number;
state: TaskInstanceState | null;
note: string | null;
};

/**
Expand Down
28 changes: 0 additions & 28 deletions airflow-core/tests/unit/api_fastapi/core_api/routes/ui/test_grid.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,6 @@
"upstream_failed": 0,
},
"end_date": None,
"note": None,
"queued_dttm": None,
"start_date": None,
"state": "success",
Expand All @@ -108,7 +107,6 @@
"upstream_failed": 0,
},
"end_date": None,
"note": None,
"queued_dttm": None,
"start_date": None,
"state": "success",
Expand All @@ -133,7 +131,6 @@
"upstream_failed": 0,
},
"end_date": None,
"note": None,
"queued_dttm": None,
"start_date": None,
"state": "success",
Expand All @@ -158,7 +155,6 @@
"upstream_failed": 0,
},
"end_date": None,
"note": None,
"queued_dttm": None,
"start_date": None,
"state": "success",
Expand All @@ -183,7 +179,6 @@
"upstream_failed": 0,
},
"end_date": None,
"note": None,
"queued_dttm": None,
"start_date": None,
"state": "success",
Expand All @@ -208,7 +203,6 @@
"upstream_failed": 0,
},
"end_date": None,
"note": None,
"queued_dttm": None,
"start_date": None,
"state": "success",
Expand All @@ -233,7 +227,6 @@
"upstream_failed": 0,
},
"end_date": None,
"note": None,
"queued_dttm": None,
"start_date": None,
"state": "success",
Expand All @@ -258,7 +251,6 @@
"upstream_failed": 0,
},
"end_date": None,
"note": None,
"queued_dttm": None,
"start_date": None,
"state": "success",
Expand Down Expand Up @@ -299,7 +291,6 @@
"upstream_failed": 0,
},
"end_date": "2024-12-30T01:02:03Z",
"note": None,
"queued_dttm": None,
"start_date": "2024-12-30T01:00:00Z",
"state": "running",
Expand All @@ -324,7 +315,6 @@
"upstream_failed": 0,
},
"end_date": None,
"note": None,
"queued_dttm": None,
"start_date": None,
"state": None,
Expand All @@ -349,7 +339,6 @@
"upstream_failed": 0,
},
"end_date": None,
"note": None,
"queued_dttm": None,
"start_date": None,
"state": None,
Expand All @@ -374,7 +363,6 @@
"upstream_failed": 0,
},
"end_date": None,
"note": None,
"queued_dttm": None,
"start_date": None,
"state": None,
Expand All @@ -399,7 +387,6 @@
"upstream_failed": 0,
},
"end_date": "2024-12-30T01:02:03Z",
"note": None,
"queued_dttm": None,
"start_date": "2024-12-30T01:00:00Z",
"state": "running",
Expand All @@ -424,7 +411,6 @@
"upstream_failed": 0,
},
"end_date": None,
"note": None,
"queued_dttm": None,
"start_date": None,
"state": "success",
Expand All @@ -449,7 +435,6 @@
"upstream_failed": 0,
},
"end_date": None,
"note": None,
"queued_dttm": None,
"start_date": None,
"state": None,
Expand All @@ -474,7 +459,6 @@
"upstream_failed": 0,
},
"end_date": None,
"note": None,
"queued_dttm": None,
"start_date": None,
"state": None,
Expand Down Expand Up @@ -797,7 +781,6 @@ def test_should_response_200_order_by(self, test_client, order_by, expected):
"upstream_failed": 0,
},
"end_date": None,
"note": None,
"queued_dttm": None,
"start_date": None,
"state": "success",
Expand All @@ -822,7 +805,6 @@ def test_should_response_200_order_by(self, test_client, order_by, expected):
"upstream_failed": 0,
},
"end_date": None,
"note": None,
"queued_dttm": None,
"start_date": None,
"state": "success",
Expand Down Expand Up @@ -852,7 +834,6 @@ def test_should_response_200_order_by(self, test_client, order_by, expected):
"upstream_failed": 0,
},
"end_date": "2024-12-30T01:02:03Z",
"note": None,
"queued_dttm": None,
"start_date": "2024-12-30T01:00:00Z",
"state": "running",
Expand All @@ -877,7 +858,6 @@ def test_should_response_200_order_by(self, test_client, order_by, expected):
"upstream_failed": 0,
},
"end_date": "2024-12-30T01:02:03Z",
"note": None,
"queued_dttm": None,
"start_date": "2024-12-30T01:00:00Z",
"state": "running",
Expand Down Expand Up @@ -916,7 +896,6 @@ def test_should_response_200_order_by(self, test_client, order_by, expected):
"upstream_failed": 0,
},
"end_date": None,
"note": None,
"queued_dttm": None,
"start_date": None,
"state": "success",
Expand All @@ -941,7 +920,6 @@ def test_should_response_200_order_by(self, test_client, order_by, expected):
"upstream_failed": 0,
},
"end_date": None,
"note": None,
"queued_dttm": None,
"start_date": None,
"state": "success",
Expand Down Expand Up @@ -971,7 +949,6 @@ def test_should_response_200_order_by(self, test_client, order_by, expected):
"upstream_failed": 0,
},
"end_date": "2024-12-30T01:02:03Z",
"note": None,
"queued_dttm": None,
"start_date": "2024-12-30T01:00:00Z",
"state": "running",
Expand All @@ -996,7 +973,6 @@ def test_should_response_200_order_by(self, test_client, order_by, expected):
"upstream_failed": 0,
},
"end_date": "2024-12-30T01:02:03Z",
"note": None,
"queued_dttm": None,
"start_date": "2024-12-30T01:00:00Z",
"state": "running",
Expand Down Expand Up @@ -1306,7 +1282,6 @@ def test_should_response_200_with_deleted_task_and_taskgroup(self, session, test
"upstream_failed": 0,
},
"end_date": None,
"note": None,
"queued_dttm": None,
"start_date": None,
"state": "success",
Expand All @@ -1331,7 +1306,6 @@ def test_should_response_200_with_deleted_task_and_taskgroup(self, session, test
"upstream_failed": 0,
},
"end_date": None,
"note": None,
"queued_dttm": None,
"start_date": None,
"state": "success",
Expand All @@ -1356,7 +1330,6 @@ def test_should_response_200_with_deleted_task_and_taskgroup(self, session, test
"upstream_failed": 0,
},
"end_date": None,
"note": None,
"queued_dttm": None,
"start_date": None,
"state": "success",
Expand Down Expand Up @@ -1396,7 +1369,6 @@ def test_should_response_200_with_deleted_task_and_taskgroup(self, session, test
"upstream_failed": 0,
},
"end_date": None,
"note": None,
"queued_dttm": None,
"start_date": None,
"state": "success",
Expand Down
Loading