Skip to content

Conversation

@Lee-W
Copy link
Member

@Lee-W Lee-W commented Mar 5, 2025

Why

We need asset ID to fetch asset details in UI

closes: #47163

What

  • fastapi
    • include asset id in asset nodes when calling "/ui/dependencies" and "/ui/structure/structure_data"
  • internal
    • Add "label" to DagDependency
      • for asset, id will be updated as asset_id, and label is set to asset_name
      • for tasks, id is still task_ide while label is set to task_display_name

^ 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.

@Lee-W Lee-W force-pushed the set-dep-id-to-asset-id-for-asset-dag-dep branch 11 times, most recently from da2032a to 9b0e3f7 Compare March 7, 2025 07:44
@Lee-W
Copy link
Member Author

Lee-W commented Mar 7, 2025

@bbovenzi this PR is somewhat blocked by #47484 (review) and will need to rework partially due to #47320. so we'll need more time to get it ready

@Lee-W Lee-W force-pushed the set-dep-id-to-asset-id-for-asset-dag-dep branch from 9b0e3f7 to 05eb7d0 Compare March 7, 2025 15:11
@Lee-W Lee-W force-pushed the set-dep-id-to-asset-id-for-asset-dag-dep branch from 05eb7d0 to 33da3c8 Compare March 10, 2025 10:49
@bbovenzi
Copy link
Contributor

bbovenzi commented Mar 11, 2025

I just noticed that DagModel.asset_expression is also missing an asset_id. We should update that too if possible

@Lee-W Lee-W force-pushed the set-dep-id-to-asset-id-for-asset-dag-dep branch 7 times, most recently from 3c12b45 to 12056bc Compare March 19, 2025 09:59
@bbovenzi bbovenzi mentioned this pull request Mar 20, 2025
@Lee-W Lee-W force-pushed the set-dep-id-to-asset-id-for-asset-dag-dep branch from 4614b33 to 7b2de24 Compare March 21, 2025 08:39
@Lee-W
Copy link
Member Author

Lee-W commented Mar 21, 2025

as the latest version resolve all the comments and this PR is approved, I'll merge it once ci passes

@Lee-W Lee-W merged commit 6233396 into apache:main Mar 21, 2025
89 checks passed
@Lee-W Lee-W deleted the set-dep-id-to-asset-id-for-asset-dag-dep branch March 21, 2025 09:41
agupta01 pushed a commit to agupta01/airflow that referenced this pull request Mar 21, 2025
…dependencies" and "/ui/structure/structure_data" (apache#47381)

* feat(serialization): change dependency_id in DagDependency with asset type to be asset_id

* feat(dag_dependency): add label

* test(serialization): fix test_derived_dag_deps_sensor

* test(serialization): fix test_dag_deps_assets_with_duplicate_asset

* test: fix test_derived_dag_deps_operator

* test(task_sdk): fix task_sdk tests due to _resolve_assets renaming

* test(api_fastapi/ui): fix test cases

* test(serialized_dag): fix test_order_of_deps_is_consistent

* feat(dot_renderer): support label

* refactor: refactor retreive_asset_models as resolve_assets_as_dag_dependencies to retreive needed info only

* feat: remove db access in serialized object

* feat(dag_dependency): improve dependency_type typing and split asset-ref to asset-name-ref and asset-uri-ref

* feat(serialized_dag): resolve dag dependency to include asset id

* refactor(serialized_dag): extract dag dependency resolving logic as a class

* test(dag_serialization): fix dag_serialization tests

* test(api_fastapi): update test cases with asset_id changes

* test(api_fastapi): fix test case with dynamic asset id

* fix(serialized_dag): fix postgrest dag dep handling

* Fix asset graph in UI

* test(api_fastapi): fix structure endpoint test case

* fix(api_fastapi): fix /ui/structure

---------

Co-authored-by: Brent Bovenzi <brent.bovenzi@gmail.com>
shubham-pyc pushed a commit to shubham-pyc/airflow that referenced this pull request Mar 22, 2025
…dependencies" and "/ui/structure/structure_data" (apache#47381)

* feat(serialization): change dependency_id in DagDependency with asset type to be asset_id

* feat(dag_dependency): add label

* test(serialization): fix test_derived_dag_deps_sensor

* test(serialization): fix test_dag_deps_assets_with_duplicate_asset

* test: fix test_derived_dag_deps_operator

* test(task_sdk): fix task_sdk tests due to _resolve_assets renaming

* test(api_fastapi/ui): fix test cases

* test(serialized_dag): fix test_order_of_deps_is_consistent

* feat(dot_renderer): support label

* refactor: refactor retreive_asset_models as resolve_assets_as_dag_dependencies to retreive needed info only

* feat: remove db access in serialized object

* feat(dag_dependency): improve dependency_type typing and split asset-ref to asset-name-ref and asset-uri-ref

* feat(serialized_dag): resolve dag dependency to include asset id

* refactor(serialized_dag): extract dag dependency resolving logic as a class

* test(dag_serialization): fix dag_serialization tests

* test(api_fastapi): update test cases with asset_id changes

* test(api_fastapi): fix test case with dynamic asset id

* fix(serialized_dag): fix postgrest dag dep handling

* Fix asset graph in UI

* test(api_fastapi): fix structure endpoint test case

* fix(api_fastapi): fix /ui/structure

---------

Co-authored-by: Brent Bovenzi <brent.bovenzi@gmail.com>
nailo2c pushed a commit to nailo2c/airflow that referenced this pull request Apr 4, 2025
…dependencies" and "/ui/structure/structure_data" (apache#47381)

* feat(serialization): change dependency_id in DagDependency with asset type to be asset_id

* feat(dag_dependency): add label

* test(serialization): fix test_derived_dag_deps_sensor

* test(serialization): fix test_dag_deps_assets_with_duplicate_asset

* test: fix test_derived_dag_deps_operator

* test(task_sdk): fix task_sdk tests due to _resolve_assets renaming

* test(api_fastapi/ui): fix test cases

* test(serialized_dag): fix test_order_of_deps_is_consistent

* feat(dot_renderer): support label

* refactor: refactor retreive_asset_models as resolve_assets_as_dag_dependencies to retreive needed info only

* feat: remove db access in serialized object

* feat(dag_dependency): improve dependency_type typing and split asset-ref to asset-name-ref and asset-uri-ref

* feat(serialized_dag): resolve dag dependency to include asset id

* refactor(serialized_dag): extract dag dependency resolving logic as a class

* test(dag_serialization): fix dag_serialization tests

* test(api_fastapi): update test cases with asset_id changes

* test(api_fastapi): fix test case with dynamic asset id

* fix(serialized_dag): fix postgrest dag dep handling

* Fix asset graph in UI

* test(api_fastapi): fix structure endpoint test case

* fix(api_fastapi): fix /ui/structure

---------

Co-authored-by: Brent Bovenzi <brent.bovenzi@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

AIP-84 Add asset.id to Dag Dependencies Graph

4 participants