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

Task Group: Node order changes after collapse #13044

Closed
JCoder01 opened this issue Dec 13, 2020 · 14 comments
Closed

Task Group: Node order changes after collapse #13044

JCoder01 opened this issue Dec 13, 2020 · 14 comments
Labels
affected_version:2.0 Issues Reported for 2.0 area:TaskGroup area:UI Related to UI/UX. For Frontend Developers. kind:bug This is a clearly a bug priority:low Bug with a simple workaround that would not block a release

Comments

@JCoder01
Copy link
Contributor

Apache Airflow version: 2.0.0rc1

Kubernetes version (if you are using kubernetes) (use kubectl version):

Environment:

  • Cloud provider or hardware configuration:
  • OS (e.g. from /etc/os-release):
  • Kernel (e.g. uname -a):
  • Install tools:
  • Others:

What happened:

The order of the nodes changes when expanding and collapsing task groups:
Before:
image

After expanding task group and collapsing:
image

What you expected to happen:
node order to be consistent
How to reproduce it:

Create a DAG with multiple task groups.
expand the top group
collapse the top group
observe is is now at the end.

Anything else we need to know:

happens every time.

@JCoder01 JCoder01 added the kind:bug This is a clearly a bug label Dec 13, 2020
@turbaszek
Copy link
Member

CC @yuqian90

@turbaszek
Copy link
Member

Thanks @JCoder01, as I understand this is not a "real bug" but just an unpleasant UX, right?

@JCoder01
Copy link
Contributor Author

@turbaszek Yeah, there is nothing wrong with the functionality. It's just a tad annoying when things move around.

@yuqian90
Copy link
Contributor

@turbaszek Yeah, there is nothing wrong with the functionality. It's just a tad annoying when things move around.

In fact, this isn't an issue for TaskGroup specifically. Even before TaskGroup was introduced, simply refreshing the page can sometimes render the Graph View differently causing the nodes to shuffle around.

I can understand the pain. However, this issue needs to be addressed in the dependent package Dagre. I think this is the open issue upstream: dagrejs/dagre#189

Unfortunately it doesn't look like they are addressing it any time soon. Hopefully someone can come up with a workaround.

@vikramkoka vikramkoka added the affected_version:2.0 Issues Reported for 2.0 label Dec 14, 2020
@ashb ashb added the area:UI Related to UI/UX. For Frontend Developers. label Dec 15, 2020
@ashb
Copy link
Member

ashb commented Dec 15, 2020

Could some test this on Rc3 -- I don't see it there.

If you do see it, please give me a reproduction test case.

@JCoder01
Copy link
Contributor Author

@ashb Just installed RC3 and see the same. The behavior is evident on that example_task_group dag.
Before
image
After
image

@ashb
Copy link
Member

ashb commented Dec 15, 2020

I wonder if I don't see it cos I use Firefox

@JCoder01
Copy link
Contributor Author

I don't know, I tried Firefox on both windows and linux and the same thing happens.

@eladkal
Copy link
Contributor

eladkal commented Dec 16, 2020

I was able to reproduce using the example_task_group DAG :

reproduce

@eladkal eladkal added the priority:low Bug with a simple workaround that would not block a release label Jan 25, 2021
@eladkal eladkal added this to the Airflow 2.0.1 milestone Jan 25, 2021
@kaxil kaxil modified the milestones: Airflow 2.0.1, Airflow 2.1 Jan 29, 2021
@jhtimmins jhtimmins modified the milestones: Airflow 2.1, Airflow 2.1.1 May 12, 2021
@kaxil
Copy link
Member

kaxil commented Jun 22, 2021

@bbovenzi Can you take a look at this one when you have time please

@kaxil kaxil modified the milestones: Airflow 2.1.1, Airflow 2.1.2 Jun 22, 2021
@bbovenzi bbovenzi removed this from the Airflow 2.1.2 milestone Jun 26, 2021
@bbovenzi
Copy link
Contributor

dagre still has not been updated. If someone wants to fork that library or add logic to remember the order, feel free to open a PR. But I will not be focusing on this as we will use a different graphing library in AIP-38.

@yuqian90
Copy link
Contributor

Hi @bbovenzi , just curious, what graphing library are we switching to in AIP-38? I think it makes sense to start thinking about how to implement Graph View and Tree View using that new library.

@bbovenzi
Copy link
Contributor

@yuqian90 I'm still looking into it, so I'm open to suggestions. We can certainly bring those updates into the current UI when it's well tested.

@bbovenzi
Copy link
Contributor

Just tested again in the new graph view. Group children appear to maintain their order.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
affected_version:2.0 Issues Reported for 2.0 area:TaskGroup area:UI Related to UI/UX. For Frontend Developers. kind:bug This is a clearly a bug priority:low Bug with a simple workaround that would not block a release
Projects
None yet
Development

No branches or pull requests

9 participants