-
Notifications
You must be signed in to change notification settings - Fork 13.7k
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
Improve DAG loading in Graph view when using Dynamic Tasks #29791
Conversation
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 Contribution Guide (https://github.com/apache/airflow/blob/main/CONTRIBUTING.rst)
|
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.
Indeed get_instance_with_map
returns a summary for all mapped task and doesn't need to be run for each index.
Tested locally and looking good :)
We might still have an issue with the edit: I just looked at the equivalent in the grid view, and we do not return a 'try_number' for mapped task summaries. (which makes sense I believe) |
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.
Nice catch! LGTM
Awesome work, congrats on your first merged pull request! |
Sounds like a good candidate for the next PR of the month ;) |
(#protm so @potiuk's nomination gets found by the automation!) Congrats on your first commit @Josersanvil 🎉 |
(cherry picked from commit 60d98a1)
(cherry picked from commit 60d98a1)
closes: #27483
This PR just makes a small change to the function in
www
that loads the DAG view for the Web UI.Right now, the view loops and retrieves the mapped instances of all the expansions of a single task, that means that if a DAG created 100 expansions of a task instance, it loops through all the 100 expansions in order to retrieve the status of the 100! This makes the loading of that view very slow, and it is also not needed, applying the same logic, we only need to retrieve the mapped instances of one of the expansions to get the status of the rest (Since they all share the same Task Id).
^ 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.rst
or{issue_number}.significant.rst
, in newsfragments.