Skip to content
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

Add task details overview page with duration chart. #46631

Merged
merged 7 commits into from
Feb 12, 2025

Conversation

tirkarthi
Copy link
Contributor

Similar to #46504 . Add widget to plot failed task instances within given logical date selected and on click goes to task instances page with state filter applied. Similar to Airflow 2 and other PR the charts are almost the same except for small differences like queued_dttm/queued_when for queued calculation and can be refactored in a later PR to avoid code duplication. Change sort to order by logical_date since the chart is constructed with logical_date along x-axis. Handle skipped tasks where queued_when is null and resort to queued duration as zero.

image

@boring-cyborg boring-cyborg bot added the area:UI Related to UI/UX. For Frontend Developers. label Feb 10, 2025
@tirkarthi tirkarthi force-pushed the task-details-overview branch from 1d307f6 to 6a5d7b9 Compare February 12, 2025 04:23
Copy link
Member

@pierrejeambrun pierrejeambrun left a comment

Choose a reason for hiding this comment

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

I think the factorization should be part of this PR. I'm not super confident merging code duplication especially when this is most of the PR content.

Other improvements / tweaks can be done later.

@tirkarthi tirkarthi force-pushed the task-details-overview branch from 6a5d7b9 to ee9141a Compare February 12, 2025 14:33
@tirkarthi
Copy link
Contributor Author

@pierrejeambrun I have refactored them to a common component DurationChart . Most of the part is same for task instance and dag run except the queued duration calculation which needs queued_when and queued_at . I am passing kind to denote the type and also using it in switch case to use the correct attribute for queued duration. I am new to JS/React refactoring. Please let me know if there is a better way. Another approach I thought of was making the component as a base class and each class TaskInstanceDurationChart and DagRunDurationChart inherits from the base class to implement queued logic duration calculation as a function for the respective class.

@tirkarthi tirkarthi force-pushed the task-details-overview branch from 06ef97d to cd75d76 Compare February 12, 2025 17:17
Copy link
Contributor

@bbovenzi bbovenzi left a comment

Choose a reason for hiding this comment

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

Just one nit on variable names.

@tirkarthi tirkarthi force-pushed the task-details-overview branch from cd75d76 to 42d3343 Compare February 12, 2025 17:44
@tirkarthi
Copy link
Contributor Author

The chart currently uses logical_date along x-axis . Since logical_date was made nullable and also not unique it should be decided how to represent multiple runs of same or null logical_date values. I feel it's out of the scope for this PR but just realized while testing with the changes to logical_date recently.

@bbovenzi
Copy link
Contributor

Yeah, I'm happy to merge now and figure out logical date later.

@bbovenzi bbovenzi merged commit c2106c4 into apache:main Feb 12, 2025
35 checks passed
@tirkarthi
Copy link
Contributor Author

Thanks @bbovenzi and @pierrejeambrun .

ambika-garg pushed a commit to ambika-garg/airflow that referenced this pull request Feb 17, 2025
* Add task details overview page with duration chart.

* Use the elements count instead total_entries which is a total count without filters and pagination applied.

* Refactor duration chart to a single component.

* Handle instances that got queued and marked with no start_date.

* Enforce types.

* Fix variable name.

* Fix rebase.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:UI Related to UI/UX. For Frontend Developers.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants