-
Notifications
You must be signed in to change notification settings - Fork 16.4k
Add producing/consuming task dependencies to AssetGraph #58059
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
base: main
Are you sure you want to change the base?
Conversation
0d11eec to
5cf9b6e
Compare
d20c28d to
5d2b047
Compare
|
Wait, so is this to show the tasks inside the Dag instead? |
|
Yes, that’s my current understanding. Based on the discussion in the issue, it seems the intention is to show the tasks inside the dag. But I’m not completely certain, so I’m happy to adjust the implementation if needed. |
5d2b047 to
7d87cfa
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.
I think we are mixing two things and it will make the graph even more complicated for users.
- On one side we have the 'dependencies' from logical execution flow / scheduling
- On the other side we have
datadependencies / lineage (which task is reading which asset event)
I'm not sure if baking those two pieces inside the same graph is a good idea. I believe it will make things hard to understand.
I assume the toggle is here for that.
Also the asset graph is really at the dag level I think. On the Dag graph on the other hand, we do have task level resolution. #51401, same for asset aliases.
Instead of having task level vs graph level, it sould be 'scheduling / execution flow' vs data dependencies
7d87cfa to
e5115c6
Compare
e5115c6 to
f87f567
Compare
f87f567 to
6ed8909
Compare
|
I've updated the implementation to split them into two views and add a button to switch between two different graph Screen.Recording.2025-12-09.at.8.06.22.PM.mov |
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 PR.
It's really nice to see this cool feature!
airflow-core/src/airflow/api_fastapi/core_api/services/ui/dependencies.py
Outdated
Show resolved
Hide resolved
airflow-core/src/airflow/api_fastapi/core_api/services/ui/dependencies.py
Outdated
Show resolved
Hide resolved
airflow-core/src/airflow/api_fastapi/core_api/routes/ui/dependencies.py
Outdated
Show resolved
Hide resolved
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.
Nit. I'm not sure we want to call that 'lineage'. That could be 'task dependencies' or something else.
I don't want people to expect a full lineage solution, while this is for now limited to "producing/consuming task dependencies"
Also for asset alias, we might need to double check. If I run asset_alias_example_alias_producer, I see:
But I don't see any consuming tasks, even after running asset_alias_example_alias_consumer. But i'm not sure if we do have the information at this point.
airflow-core/src/airflow/api_fastapi/core_api/services/ui/dependencies.py
Outdated
Show resolved
Hide resolved
airflow-core/tests/unit/api_fastapi/core_api/routes/ui/test_dependencies.py
Show resolved
Hide resolved
4763e2c to
b2aadce
Compare
8fe8358 to
0dc4ba8
Compare
671d7b1 to
bf03b37
Compare
bbovenzi
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.
Looking much better.
In another PR, I think we should update the assets list and asset details pages to add scheduled as a column in the table or header card.
bf03b37 to
53ab31b
Compare
Make sense to me. I could do that in a follow up. |
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.
LGTM, we can improve with follow up PRs as suggested, I think it's a good base.
airflow-core/src/airflow/api_fastapi/core_api/routes/ui/dependencies.py
Outdated
Show resolved
Hide resolved
…encies.py Co-authored-by: Jason(Zhe-You) Liu <68415893+jason810496@users.noreply.github.com>
53ab31b to
34a5eda
Compare

Related Issue
Closes #50873
Why
How
demo.mov
^ 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.