-
Notifications
You must be signed in to change notification settings - Fork 16.3k
Fix task instance display in task group grid view #55670
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.
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
Fix task instance display in task group grid view #55670
Conversation
3397e14 to
8a70414
Compare
pierrejeambrun
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the PR.
A few comments to address.
For 1 - I think it's nice just a few comments but we're almost there.
For 2 - I think there is a problem. Basically the issue comes from prefix_group_id=False in the group definition of the Dag. And could be also caused by overriding the display_name of tasks contained in the group. Because we are searching over task_display_name and the only way it will match is if the group id prefix is contained in the field we are filtering on the db side. The implementation you made here is not great because it's not handled on the db side but use the serialized dag and also because it mixes 'task_display_namesearch andtask_id` search. I would split this PR and move 2 - changes to a different PR so it doesn't block this one.
airflow-core/src/airflow/api_fastapi/core_api/routes/public/task_instances.py
Outdated
Show resolved
Hide resolved
airflow-core/src/airflow/api_fastapi/core_api/routes/public/task_instances.py
Outdated
Show resolved
Hide resolved
airflow-core/src/airflow/api_fastapi/core_api/routes/public/task_instances.py
Outdated
Show resolved
Hide resolved
airflow-core/src/airflow/api_fastapi/core_api/services/ui/task_group.py
Outdated
Show resolved
Hide resolved
airflow-core/src/airflow/api_fastapi/core_api/services/ui/grid.py
Outdated
Show resolved
Hide resolved
pierrejeambrun
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@yimingpeng Any chance you can address the comments so we can go ahead and merge resolution for 1?
|
Hi @pierrejeambrun , yes and sorry, a bit busy lately, will work on this soonish, thanks 🙏 |
463273b to
f0b38a4
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking nice, just one nit and we should be good to go.
jason810496
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the fix!
f0b38a4 to
e3f1276
Compare
…55670) (cherry picked from commit 07d2892) Co-authored-by: Yiming Peng <yimingpengjojo@gmail.com> closes: apache#55319
closes: #55319
Problem Summary
When checking the status of a task group in the grid view:
3 tasksinstead of40 tasks).Proposed Solution
For Issue 1
child_states(mapped tasks), now we expand and count each state of individual task instances rather than treating them as one abstract task### For Issue 2- First we check whether the task name pattern is a task group ID- Next we resolve this pattern to all task ids within the group- So whenever we encounter a task group, we query task instances by filtering on the task ids extracted from last step other than using the original name pattern.After the Fix
Screen.Recording.2025-09-17.at.9.04.56.PM.mp4
Running result on 2025-10-29
^ 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.