-
Notifications
You must be signed in to change notification settings - Fork 16.3k
fix: Enable real-time extra links updates for TriggerDagRunOperator #59507
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: Enable real-time extra links updates for TriggerDagRunOperator #59507
Conversation
- Add smart polling to ExtraLinks component using useAutoRefresh hook - Poll automatically when DAG is active and links are not yet available - Stop polling once extra links appear to reduce server load - Respects global auto_refresh_interval config and DAG paused state - Fixes issue where Triggered DAG button only appeared after manual refresh Closes apache#58928
|
Congratulations on your first Pull Request and welcome to the Apache Airflow community! If you have any issues or are unsure about any anything please check our Contributors' Guide (https://github.com/apache/airflow/blob/main/contributing-docs/README.rst)
|
airflow-core/src/airflow/ui/src/pages/TaskInstance/ExtraLinks.tsx
Outdated
Show resolved
Hide resolved
- Add task metadata check using useTaskServiceGetTask - Only poll if task.extra_links.length > 0 - Conditionally render ExtraLinks component in Details.tsx - Prevents unnecessary API calls for tasks without extra links Addresses review feedback from @bbovenzi
|
@bbovenzi I've addressed both suggestions:
This prevents:
The component now only renders and polls when appropriate. Thanks for the thorough review! |
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.
An easy way is to re-use the same parent refetchInterval: (query) => (isStatePending(query.state.data?.state) ? refetchInterval : false),
That is based on the 'try' 'pending' status. You can give that to the child component so it will refresh as long as the try isn't settled. Which might save you one 'useTaskServiceGetTask` call as well.
- Pass parent's refetchInterval to ExtraLinks and BlockingDeps components - Remove duplicate useAutoRefresh and useTaskServiceGetTask calls - Eliminate invalid taskInstance.extra_links property access (doesn't exist on TaskInstanceResponse) - Polling now tied to try instance pending state - stops automatically when task completes - Simplifies architecture by centralizing refresh logic in parent component Addresses review feedback from @pierrejeambrun and @bbovenzi
|
@pierrejeambrun Thank you for the feedback! I've implemented both of your suggestions: Reused parent's refetchInterval pattern As you suggested, I'm now passing the same refetch logic to child components based on the try instance's pending state. Both ExtraLinks and BlockingDeps receive isStatePending(tryInstance?.state) ? refetchInterval : false as a prop. This eliminated the useTaskServiceGetTask call from ExtraLinks and removed duplicate useAutoRefresh hooks. Polling now automatically stops when the try instance is no longer pending. Fixed invalid property access Removed the conditional check for taskInstance?.extra_links - you're absolutely right that this property doesn't exist on TaskInstanceResponse. The ExtraLinks component now renders unconditionally and handles its own empty state internally. The architecture is much cleaner now with a single source of truth for refresh logic in the parent component. I also applied the same pattern to BlockingDeps for consistency so it benefits from the same polling behavior. Ready for another review! |
- Reorder type union to alphabetical order (number | false) - Format ExtraLinks component to single line
|
Awesome work, congrats on your first merged pull request! You are invited to check our Issue Tracker for additional contributions. |
…unOperator (#59507) * fix: Enable real-time extra links updates for TriggerDagRunOperator - Add smart polling to ExtraLinks component using useAutoRefresh hook - Poll automatically when DAG is active and links are not yet available - Stop polling once extra links appear to reduce server load - Respects global auto_refresh_interval config and DAG paused state - Fixes issue where Triggered DAG button only appeared after manual refresh Closes #58928 * fix: prevent infinite polling for tasks without extra links - Add task metadata check using useTaskServiceGetTask - Only poll if task.extra_links.length > 0 - Conditionally render ExtraLinks component in Details.tsx - Prevents unnecessary API calls for tasks without extra links Addresses review feedback from @bbovenzi * refactor: reuse parent refetchInterval pattern in child components - Pass parent's refetchInterval to ExtraLinks and BlockingDeps components - Remove duplicate useAutoRefresh and useTaskServiceGetTask calls - Eliminate invalid taskInstance.extra_links property access (doesn't exist on TaskInstanceResponse) - Polling now tied to try instance pending state - stops automatically when task completes - Simplifies architecture by centralizing refresh logic in parent component Addresses review feedback from @pierrejeambrun and @bbovenzi * style: apply pre-commit hook formatting fixes - Reorder type union to alphabetical order (number | false) - Format ExtraLinks component to single line (cherry picked from commit 2768fba) Co-authored-by: subhash-0000 <122723782+subhash-0000@users.noreply.github.com>
…unOperator (#59507) (#60225) * fix: Enable real-time extra links updates for TriggerDagRunOperator - Add smart polling to ExtraLinks component using useAutoRefresh hook - Poll automatically when DAG is active and links are not yet available - Stop polling once extra links appear to reduce server load - Respects global auto_refresh_interval config and DAG paused state - Fixes issue where Triggered DAG button only appeared after manual refresh Closes #58928 * fix: prevent infinite polling for tasks without extra links - Add task metadata check using useTaskServiceGetTask - Only poll if task.extra_links.length > 0 - Conditionally render ExtraLinks component in Details.tsx - Prevents unnecessary API calls for tasks without extra links Addresses review feedback from @bbovenzi * refactor: reuse parent refetchInterval pattern in child components - Pass parent's refetchInterval to ExtraLinks and BlockingDeps components - Remove duplicate useAutoRefresh and useTaskServiceGetTask calls - Eliminate invalid taskInstance.extra_links property access (doesn't exist on TaskInstanceResponse) - Polling now tied to try instance pending state - stops automatically when task completes - Simplifies architecture by centralizing refresh logic in parent component Addresses review feedback from @pierrejeambrun and @bbovenzi * style: apply pre-commit hook formatting fixes - Reorder type union to alphabetical order (number | false) - Format ExtraLinks component to single line (cherry picked from commit 2768fba) Co-authored-by: subhash-0000 <122723782+subhash-0000@users.noreply.github.com>
…pache#59507) * fix: Enable real-time extra links updates for TriggerDagRunOperator - Add smart polling to ExtraLinks component using useAutoRefresh hook - Poll automatically when DAG is active and links are not yet available - Stop polling once extra links appear to reduce server load - Respects global auto_refresh_interval config and DAG paused state - Fixes issue where Triggered DAG button only appeared after manual refresh Closes apache#58928 * fix: prevent infinite polling for tasks without extra links - Add task metadata check using useTaskServiceGetTask - Only poll if task.extra_links.length > 0 - Conditionally render ExtraLinks component in Details.tsx - Prevents unnecessary API calls for tasks without extra links Addresses review feedback from @bbovenzi * refactor: reuse parent refetchInterval pattern in child components - Pass parent's refetchInterval to ExtraLinks and BlockingDeps components - Remove duplicate useAutoRefresh and useTaskServiceGetTask calls - Eliminate invalid taskInstance.extra_links property access (doesn't exist on TaskInstanceResponse) - Polling now tied to try instance pending state - stops automatically when task completes - Simplifies architecture by centralizing refresh logic in parent component Addresses review feedback from @pierrejeambrun and @bbovenzi * style: apply pre-commit hook formatting fixes - Reorder type union to alphabetical order (number | false) - Format ExtraLinks component to single line
…pache#59507) * fix: Enable real-time extra links updates for TriggerDagRunOperator - Add smart polling to ExtraLinks component using useAutoRefresh hook - Poll automatically when DAG is active and links are not yet available - Stop polling once extra links appear to reduce server load - Respects global auto_refresh_interval config and DAG paused state - Fixes issue where Triggered DAG button only appeared after manual refresh Closes apache#58928 * fix: prevent infinite polling for tasks without extra links - Add task metadata check using useTaskServiceGetTask - Only poll if task.extra_links.length > 0 - Conditionally render ExtraLinks component in Details.tsx - Prevents unnecessary API calls for tasks without extra links Addresses review feedback from @bbovenzi * refactor: reuse parent refetchInterval pattern in child components - Pass parent's refetchInterval to ExtraLinks and BlockingDeps components - Remove duplicate useAutoRefresh and useTaskServiceGetTask calls - Eliminate invalid taskInstance.extra_links property access (doesn't exist on TaskInstanceResponse) - Polling now tied to try instance pending state - stops automatically when task completes - Simplifies architecture by centralizing refresh logic in parent component Addresses review feedback from @pierrejeambrun and @bbovenzi * style: apply pre-commit hook formatting fixes - Reorder type union to alphabetical order (number | false) - Format ExtraLinks component to single line
Fixes #58928
The "Triggered DAG" button from
TriggerDagRunOperatornow appears in real-time without requiring a manual page refresh.Problem
Previously, extra links (including the "Triggered DAG" button) only appeared after:
This reduced the utility of extra links, especially for monitoring triggered child DAGs in real-time.
Solution
ExtraLinkscomponent using the existinguseAutoRefreshhookauto_refresh_intervalconfig and DAG paused stateChanges Made
airflow-core/src/airflow/ui/src/pages/TaskInstance/ExtraLinks.tsxrefetchIntervallogic with conditional pollingDetails.tsxfor polling task instancesTesting
Code Quality:
vite buildpasses)useAutoRefreshhook)Manual Testing Checklist (for reviewers):
breeze start-airflow --dev-modeTriggerDagRunOperatorPerformance Impact
✅ Positive: Polling stops when links appear, actually reducing server load compared to continuous polling
✅ Respects
auto_refresh_intervalconfig (user-configurable)✅ Only polls when DAG is active (not paused)
Related
This fix uses the same auto-refresh pattern already established in
Details.tsxfor task instance polling, ensuring consistency and maintainability across the codebase.Type: Bug Fix
Component: UI (React)
Breaking Changes: None
New Dependencies: None