Hierarchical GScan

[kinow]

Related to #139

• Brings hierarchy into GScan, using existing Tree, TreeItem, Job, and Task components.
• Creates modules for sort, filter (will spread this pattern to other components over next PR's/days/weeks/...).
• Makes the filter workflow name field case-insensitive (i.e. searching for RUN1 will include a workflow like five/run1).
• Has a known bug in sorting by status, see comment below, and pending Fix sorting in GScan due to strange enumify behaviour #703
• I think I could try some performance improvements, for instance, for every delta received it goes over the tree, finds the next insertion index, filters, etc... if you have 40 workflows, you will receive 40 workflows. We could debounce it and run this every second or every 500ms, giving time for more deltas to be applied and reduce computation in JS and DOM updates (confirmed the DOM is actually updated while the deltas are applied if not super-fast 😥 ). But this is for a next PR, maybe 👍

Requirements check-list

• I have read CONTRIBUTING.md and added my name as a Code Contributor.
• Contains logically grouped changes (else tidy your branch by rebase).
• Does not contain off-topic changes (use other PRs for other changes).
• Appropriate tests are included (unit and/or functional).
• Appropriate change log entry included.
• No documentation update required.

Note for reviewers: you may find workflows that are not sorted correctly by status. This is a known issue, with a fix already in review. This is what it may look like (run4 is supposed to go up I think, as it is running, but Enumify is messing up).

p.s.: to test calling the callback for a random delta, execute the following in the browser console

var deltas = [
{
  "workflow": {
    "id": "user|research/test/one/run2",
    "name": "research/test/one/run2",
    "status": "running",
    "owner": "user",
    "host": "localhost",
    "port": 43079,
    "stateTotals": {
      "waiting": 1,
      "expired": 0,
      "preparing": 0,
      "submit-failed": 0,
      "submitted": 3,
      "running": 1,
      "failed": 0,
      "succeeded": 0
    },
    "latestStateTasks": {
      "running": [
        "running-task"
      ]
    }
  }
},
{
  "workflow": {
    "id": "user|research/test/two/run1",
    "name": "research/test/two/run1",
    "status": "running",
    "owner": "user",
    "host": "localhost",
    "port": 43079,
    "stateTotals": {
      "waiting": 1,
      "expired": 0,
      "preparing": 0,
      "submit-failed": 0,
      "submitted": 3,
      "running": 1,
      "failed": 0,
      "succeeded": 0
    },
    "latestStateTasks": {
      "running": [
        "running-task"
      ]
    }
  }
},
{
  "workflow": {
    "id": "research",
    "name": "research",
    "status": "running",
    "owner": "user",
    "host": "localhost",
    "port": 43079,
    "stateTotals": {
      "waiting": 1,
      "expired": 0,
      "preparing": 0,
      "submit-failed": 0,
      "submitted": 3,
      "running": 1,
      "failed": 0,
      "succeeded": 0
    },
    "latestStateTasks": {
      "running": [
        "running-task"
      ]
    }
  }
}
]
deltas.forEach((delta) => {
window.app.$store.dispatch('workflows/applyWorkflowsDeltas', { deltas: { added: delta } })
})

[codecov-commenter]

Codecov Report

Merging #502 (3c31fac) into master (309a6d5) will increase coverage by 0.44%.
The diff coverage is 98.98%.

@@            Coverage Diff             @@
##           master     #502      +/-   ##
==========================================
+ Coverage   89.96%   90.41%   +0.44%     
==========================================
  Files          79       83       +4     
  Lines        1535     1575      +40     
  Branches      112      105       -7     
==========================================
+ Hits         1381     1424      +43     
+ Misses        122      121       -1     
+ Partials       32       30       -2     

Flag	Coverage Δ
e2e	77.77% <78.78%> (-0.21%)	⬇️
unittests	80.66% <95.95%> (+0.74%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
src/components/cylc/tree/index.js	100.00% <ø> (+0.66%)	⬆️
src/components/cylc/tree/nodes.js	100.00% <ø> (ø)
src/store/workflows.module.js	89.47% <ø> (ø)
src/components/cylc/tree/sort.js	80.00% <80.00%> (ø)
src/components/cylc/common/sort.js	100.00% <100.00%> (ø)
src/components/cylc/gscan/GScan.vue	90.62% <100.00%> (+0.96%)	⬆️
src/components/cylc/gscan/filters.js	100.00% <100.00%> (ø)
src/components/cylc/gscan/nodes.js	100.00% <100.00%> (ø)
src/components/cylc/gscan/sort.js	100.00% <100.00%> (ø)
... and 3 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 309a6d5...3c31fac. Read the comment docs.

[kinow]

This new feature doesn't look too complicated to add, but I would like to see, first, if we can add it using the existing Tree and TreeItem components.

For that, we just need to make Tree and TreeItem as generic as possible. And also need a preprocessing step, to occur after the data is received, and before it is given to the GScan component, so that we have the hierarchy for workflows with slashes in their names.

I created two workflows to experiment with, the a/1, and a/b/c.

For now I am relying on five to let me modify the Tree and TreeItem without breaking the existing features. For example:

• added a prop filterable to the Tree component, so we can disable the filters in GScan (later we can check if it would be possible to pass a different filter?).
• added slots to TreeItem. The most important slot is <slot name="node">. This slot is the slot used for tree items that are not in ['cyclepoint', 'family-proxy', 'task-proxy', 'job'] (e.g. workflow nodes will use this slot 👍)

WIP an "after" screenshot coming soon, once I finish this PoC

[kinow]

Huh, not so straightforward 😆

But so far less than 2 hours spent on this, so not too bad. Will continue tomorrow.

[kinow]

Getting there!

[kinow]

In the image above, the links to workflows are working. It is the UI with the latest commit here. It is missing to group hierarchies that have common parts (e.g. a/1 and a/b/c share the a part, so they should have a common node starting the tree branch).

But what happens when we have running workflows in these hierarchies? At the moment we show running workflows at the top. But if we have a workflow like a/1 running, and we moved the a node to the top, it could have a/b/c and several other workflows stopped under it too.

So would we still display that hierarchy at the top? Along with all other stopped workflows? And below it display other running workflows like five? Followed by stopped workflows?

[hjoliver]

Good question. We may need to be able to choose between a flat list with "global" sorting (of running/not-running) and a tree view with partial sorting (by running/not-running)

A flat list, particularly when combined with filtering, is easiest to use if you don't care about the hierarchical grouping.

A tree view can't be fully sorted like a flat list without duplication of branches, which I don't think we would want.

I guess as a start, the tree view should sort (by running/not-running) in the leaves on each terminal branch.

And we could also consider sorting branches so that those with one or more running flows are at the top, and those with no running flows are at the bottom.

That's probably the best we can do, for sorting within a tree (is it?)?

[oliver-sanders]

Note we used to offer a flat option in the tree view for much the same reason (better re-implemented as a table if people really want that). So long as hierarchical is the default!

Note that hierarchical display makes more sense if we go down the hierarchical run registration route i.e:

~/
    cylc-run/
        flow-name/
            run1/
            run2/

Also, with this model the flow-name node could be used to represent the "template" suite providing a workspace where users could graph, validate and edit the Rose/Cylc configuration before running it.

[hjoliver]

@kinow I've had a quick play with this. First thing that struck me was, the hierarchy is being displayed as separate hierarchical items rather than as a unified tree form:

(The root and branches should merge going up the tree, right?)

[hjoliver]

I see you commented on that yourself above, but maybe my answer wasn't very helpful. I was trying to say we can't have both a grouping tree and absolute sorting of leaves by running state. And I went off on a tangent a bit by saying we might also need a flat (non-tree) gscan with absolute sorting by state - but that's a potential addition for the future. For now, we definitely want a tree gscan. The gscan tree will have multiple trunks (i.e. multiple trees - like the multiple cycle point trunks in the tree view). So we should sort trunks according to whether or not any of their leaf flows are running: those with any running flows at the top; those with no running flows at the bottom. And similarly inside the lower branches of any tree, down to the leaves (i.e. running leaves at the top of their own expanded tree, stopped ones at the bottom of it).

Does that make sense?

[kinow]

(The root and branches should merge going up the tree, right?)

Yup, I was working on that when I realized it would break sorting.

From what I understood in the discussion here, it's OK to go with hierarchy, and sort by the complete name.

So if we have a/b/c and aaa and c/b/a, the order would simply be aaa, then a/b/c and then c/b/a. i.e. the sort compares the string a/b/c, not the workflow name (c in this case).

Also, filtering would be by workflow name, so it would use a/b/c and any string like "a", "b", "c", would not remove the workflow from the list.

And then later we can add a toggle to switch between flat and non-flat. Or would it be best to do that now? 🤔

[hjoliver]

(Alphanumeric sorting might be wanted too, but that can be another PR; this one should only have alphanumeric sorting among groups of running or stopped flows, I guess).

[hjoliver]

And then later we can add a toggle to switch between flat and non-flat. Or would it be best to do that now?

A follow-up is fine unless it's really easy for you.

From what I understood in the discussion here, it's OK to go with hierarchy, and sort by the complete name.

Well I was suggesting a combination of that and run/stopped sort.

Trunks with any running flows under them should go above those with no running flows. Within that, sort by name. And if you completely expand one trunk, running leaf-flows at the top and stopped ones below them - but sort all the running flows within that group by name, and all the stopped ones within that group by name.

[hjoliver]

(If that's still not very clear let's have a chat tomorrow - it's hard to explain this stuff in text! - I have to go out now to a school thing...)

[hjoliver]

I guess I'm just talking about primary and secondary sort keys. Primary = running/stopped; secondary = alphanumeric. 💥
And that's for all the trees at the top level; and for intermediate branches within a tree (if expanded); and for leaves within a tree (if expanded).

Users might like plain alphanumeric sorting too - if they are looking to start stopped flows, or are looking for a particular flow whether or not it is currently running. Maybe that would be easiest for the first edition of this PR? And follow-up to allow choice of different sorting including by running/stopped state within each tree/trunk.

[kinow]

Righto. I will try to implement as close as possible, but writing as little code as possible, so that and the others can take a look (some times looking at the running UI more ideas appear, or we change our mind), and also for me to take a look at performance. I remember having some issues when implementing hierarchy, and that was before the offline workflows I think? So now I will have even more workflows.

The first thing I will do is find a way to merge the families. So a/1 and a/2 both appear under a single a node 👍 then will start working on the sorting.

[hjoliver]

One thing that definitely won’t change is we don’t want to break the “families” up. Sorting, by whichever keys, can only change the order of the top level trees, and the order of nodes within expanded trees.

[kinow]

Whew rebased. Wrote down a few post-its & scribbles as I re-read the old code with some problems & improvements after the rebase. Will start working on these items tomorrow please ignore CI for now.

It is interesting to note that our improvements with Vuex, subscriptions, and code-reviewing GScan simplified the code a lot. We had 4 files changed, multiple lines. Now if you look at the diff, just 2 files, with a few lines modified :-) slowly the design of the UI project is stabilizing and getting easier to add new code & maintain.

[kinow]

Moved changelog to 0.6

[hjoliver]

Looks good, tests as working 👍

As a follow-up, we should have a summary state propagate up the tree (so you can see if anything is running in a collapsed branch...)

[kinow]

As a follow-up, we should have a summary state propagate up the tree (so you can see if anything is running in a collapsed branch...)

#721

[oliver-sanders]

Looks great, thanks!

One issue for follow-up, hopefully small although it might be difficult due to component structure? The hover/selection is indented and doesn't stretch right up to the right-hand margin. This can get a little messy for hierarchically named workflows.

[kinow]

Looks great, thanks!

One issue for follow-up, hopefully small although it might be difficult due to component structure? The hover/selection is indented and doesn't stretch right up to the right-hand margin. This can get a little messy for hierarchically named workflows.

Ah, hadn't notice it! #730 Thanks!