Skip to content

Conversation

@choo121600
Copy link
Member

@choo121600 choo121600 commented May 14, 2025

This PR integrates the owner_links metadata into the DAG Header UI component. #48562

Screen.Recording.2025-05-15.at.10.09.56.AM.mov
Screen.Recording.2025-05-16.at.11.05.38.AM.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.rst or {issue_number}.significant.rst, in airflow-core/newsfragments.

@boring-cyborg boring-cyborg bot added the area:UI Related to UI/UX. For Frontend Developers. label May 14, 2025
@choo121600 choo121600 changed the title AIP-38: Add support for owner_links in DAG Header UI #48562 AIP-38: Add support for owner_links in DAG Header UI May 14, 2025
@pierrejeambrun pierrejeambrun added this to the Airflow 3.1.0 milestone May 15, 2025
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.

Thanks for the PR 🎉

A few suggestions

@bbovenzi
Copy link
Contributor

Can you provide a screenshot of a Dag with multiple owners too? I think we will need the same comma separation and "+ X more" component that we use for Tags

@choo121600
Copy link
Member Author

Can you provide a screenshot of a Dag with multiple owners too? I think we will need the same comma separation and "+ X more" component that we use for Tags

Would it make sense to reuse the LimitedItemList component that we use for Tags for displaying DAG owners as well?
Also, do you think it’s appropriate to set the maxItems to 3, same as Tags?

@bbovenzi
Copy link
Contributor

Can you provide a screenshot of a Dag with multiple owners too? I think we will need the same comma separation and "+ X more" component that we use for Tags

Would it make sense to reuse the LimitedItemList component that we use for Tags for displaying DAG owners as well? Also, do you think it’s appropriate to set the maxItems to 3, same as Tags?

Yes and yes

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.

Overall LGTM, just one comment

@choo121600 choo121600 requested a review from pierrejeambrun May 17, 2025 20:14
@bbovenzi bbovenzi merged commit dedf4df into apache:main May 19, 2025
42 checks passed
dadonnelly316 pushed a commit to dadonnelly316/airflow that referenced this pull request May 26, 2025
* feat: Add owner_links field to DAGDetailsResponse model and update related schemas

* feat: add owner_links to DAG Header owner section

* remove owner_links in test code

* add tests for DAG owner_links in DAG details endpoint

* refactor: apply ruff formatting to test_dags.py

* refactor: simplify DagOwners component

* refactor: improve separator logic in DagOwners component

* refactor: apply LimitedItemsList for cleaner item rendering logic

* refactor: enhance Link component attributes
sanederchik pushed a commit to sanederchik/airflow that referenced this pull request Jun 7, 2025
* feat: Add owner_links field to DAGDetailsResponse model and update related schemas

* feat: add owner_links to DAG Header owner section

* remove owner_links in test code

* add tests for DAG owner_links in DAG details endpoint

* refactor: apply ruff formatting to test_dags.py

* refactor: simplify DagOwners component

* refactor: improve separator logic in DagOwners component

* refactor: apply LimitedItemsList for cleaner item rendering logic

* refactor: enhance Link component attributes
@choo121600 choo121600 deleted the feature/owner-links-ui branch June 12, 2025 02:00
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