Skip to content
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 for TaskGroup toggles for duplicated labels #34072

Merged

Conversation

Usiel
Copy link
Contributor

@Usiel Usiel commented Sep 4, 2023

closes: #34066

We have been using _.label so far to remember which task groups are opened/closed. Unfortunately, this leads to issues when the same label is used twice, which can happen as described in the linked issue. With this change we switch to use _.id instead, which must be unique (otherwise parsing the DAG would lead to DuplicateTaskIdFound).

New behavior (compare with issue's gif):
image

I extended the existing tests for the grid view to check for the new behavior. However, it doesn't look like we have any tests for the new graph view yet. I can give it a shot and create some basic tests (also covering the task group toggle).


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

We have been using `_.label` so far to remember which task groups are opened/closed. Unfortunately, this leads to issues when the same label is used twice, which can happen as described in the linked issue.
With this change we switch to use `_.id` instead, which must be unique (otherwise parsing the DAG would lead to `DuplicateTaskIdFound`).

Fixes apache#34066
@boring-cyborg boring-cyborg bot added area:UI Related to UI/UX. For Frontend Developers. area:webserver Webserver related Issues labels Sep 4, 2023
Copy link
Contributor

@bbovenzi bbovenzi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch! I must've missed putting this back to id

@bbovenzi bbovenzi added this to the Airflow 2.7.2 milestone Sep 6, 2023
@bbovenzi bbovenzi merged commit b9acffa into apache:main Sep 6, 2023
42 checks passed
@ephraimbuddy ephraimbuddy added the type:bug-fix Changelog: Bug Fixes label Oct 3, 2023
ephraimbuddy pushed a commit that referenced this pull request Oct 5, 2023
* Fix for TaskGroup toggles for duplicated labels

We have been using `_.label` so far to remember which task groups are opened/closed. Unfortunately, this leads to issues when the same label is used twice, which can happen as described in the linked issue.
With this change we switch to use `_.id` instead, which must be unique (otherwise parsing the DAG would lead to `DuplicateTaskIdFound`).

Fixes #34066

* Fix toggle for Gantt as well

(cherry picked from commit b9acffa)
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. area:webserver Webserver related Issues type:bug-fix Changelog: Bug Fixes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Toggling TaskGroup toggles all TaskGroups with the same label on Graph/Grid
3 participants