Auto-collapse single run-dir workflows

[AaronDCole]

Sorry for the mess-around guys.

This is (once again) a MR to collapse the single child workflows in the gscan component + fixed some unit test that were causing me grief.

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).
• Already covered by existing tests.
• Does not need tests (why?).

• Appropriate change log entry included.
• No change log entry required (why? e.g. invisible to users).

• I have opened a documentation PR at cylc/cylc-doc/pull/XXXX.
• No documentation update required.

[hjoliver]

To test this I installed and ran multiple instances of a workflow, one with one run dir, and one with two; and the same again with an additional level in the hierarchy:

for N in cat dog dog top/cat top/dog top/dog; do
  cylc install --workflow-name=$N
  cylc play $N
done 

Result:

$ tree -d -L 3 ~/cylc-run/ | grep -E -v '(log|share|work|cylc)'
├── cat
│   ├── run1
│   └── runN -> run1
├── dog
│   ├── run1
│   ├── run2
│   └── runN -> run2
└── top
    ├── cat
    │   ├── run1
    │   └── runN -> run1
    └── dog
        ├── run1
        ├── run2
        └── runN -> run2

Via the UI:

So the collapse/expand seems to work well, and it looks great, but the state summary propagates upward only if there is a single child workflow, and then only up one level.

But ...

to collapse the single child workflows

... maybe that indicates this was intentional, as a first cut @AaronDCole ?

(And finally: cylc stop '*' to stop them all at once).

[hjoliver]

With most collapsed:

[hjoliver]

@AaronDCole - I'm now getting a summary when there are multiple workflows at the same level, but the summary seems to be that of a single one of them, rather than a combined summary?

[hjoliver]

@AaronDCole - what's the status of this one now? It looks like you've pushed commits since my review comments above, but you haven't commented on what changed...

[hjoliver]

@AaronDCole - I checked out the branch and tested it, looking good.

• autocollapse when there's a single run-dir works great 👍
• summaries are displayed at the workflow name level even if there are multiple run-dirs 👍

However, the summaries:

• should display on a background of greyed-out job icons, just as they are at the fully expanded level
• should aggregate the jobs states in all of the workflows below them (in my test with foo/run1 and foo/run2, tooltips over the summary icons at "foo" showed the same data as foo/run1 (i.e. foo/run2 did not contribute)

I need to remind myself of what form the summary data is - not sure if it can be easily aggregated like that?

(For others, propagation of state summaries to higher levels - e.g. to a in a/b/run3 can be follow-up work)

[hjoliver]

BTW: if it's not so easy to get the multi-run-dir summaries working (requires combining summary data from multiple workflows) you could punt that to follow-up work. Here, you could just to do the single-run-dir collapse summaries (as per the title of this PR) and not show a summary at all for workflows with more than one run-dir.

[AaronDCole]

@AaronDCole - I checked out the branch and tested it, looking good.

• autocollapse when there's a single run-dir works great 👍
• summaries are displayed at the workflow name level even if there are multiple run-dirs 👍

However, the summaries:

• should display on a background of greyed-out job icons, just as they are at the fully expanded level
• should aggregate the jobs states in all of the workflows below them (in my test with foo/run1 and foo/run2, tooltips over the summary icons at "foo" showed the same data as foo/run1 (i.e. foo/run2 did not contribute)

I need to remind myself of what form the summary data is - not sure if it can be easily aggregated like that?

(For others, propagation of state summaries to higher levels - e.g. to a in a/b/run3 can be follow-up work)

I think the reason I dont show all the job icons including the greyed out ones, is that when you combine the single run name with the task title below it (aka something/something), it tends to overrun the available space. This wont be such an issue once the resizable drawer is merged however.

I may have to push the aggregate job tasks to another ticket, but Im investigating currently

[AaronDCole]

@hjoliver Ive just pushed an update to this which hopefully adds the missing features from last time, it now shows the task state summaries at every level (and fixed an issue with the-child workflow button not working at the level above it).

[hjoliver]

Thanks @AaronDCole - taking a look at this tomorrow.

[MetRonnie]

The icons for stopped multilevel workflows are not grey like the one-level ones. Also, if there are more than 2 levels, the icon is a question mark.

If you click on the question mark, you can see that the reason it's a question mark is because it's not the full workflow ID, and there is no workflow a/b, only a/b/c

[AaronDCole]

The icons for stopped multilevel workflows are not grey like the one-level ones. Also, if there are more than 2 levels, the icon is a question mark.

If you click on the question mark, you can see that the reason it's a question mark is because it's not the full workflow ID, and there is no workflow a/b, only a/b/c

Looking into these comments now, Im aiming to have this closed up by the end of the week

[AaronDCole]

The icons for stopped multilevel workflows are not grey like the one-level ones. Also, if there are more than 2 levels, the icon is a question mark.


If you click on the question mark, you can see that the reason it's a question mark is because it's not the full workflow ID, and there is no workflow a/b, only a/b/c

Looking into these comments now, Im aiming to have this closed up by the end of the week

@hjoliver @oliver-sanders

I made a change that removes the button entirely for 2ed or 3ed level nested levels as (as you said) it doesn't actually refer to a real workflow - but on reflection is that something we want?

Currently, the choice for whether to display that button looks at how many children nodes there are, and in the case there is only one, it will display (which works in cases where its looking at a single-valid-child), but Oliver has correctly pointed out that a single sub folder will be viewed the same way (although the folder is not in itself a workflow), which will obviously cause the question mark.

We are referencing the child workflow rather arbitrary (scope.node.children[0].node), if we want to allow infinite nesting where the workflow icon always refers to the (valid) child workflow, we will need a more eloquent way of doing this, e.g. getNextValidWorkflow(scope.node) or something similar.

Let me know if I need to explain this better, but hopefully you get what I mean. Should the workflow-icon (button) only show if the direct child is a valid workflow (aka not show in cases where there is a single FOLDER child), or should it transcend the tree limitation and show on every level where there is only one child (provided there is an eventual single child, although that might become problematic to work out).

[oliver-sanders]

If you click on the question mark, you can see that the reason it's a question mark is because it's not the full workflow ID, and there is no workflow a/b, only a/b/c

Looking into these comments now, Im aiming to have this closed up by the end of the week

The new data store which automatically creates intermediate nodes (e.g. ~a/b for ~a/b/c//) which may help here.

I think this PR should be fairly easy to reconcile with the data store work as there's minimal overlap. The main changes which may impact are:

• workflow-name-part has changed to workflow-part.
• <GScan>.workflowNodes has changed to <GScan>.workflows.
• node.node.name has changed to node.name.
• Default expanded nodes are now configured by type (i.e. workflow-part).

Happy to help as needed whichever way around they're merged.

[oliver-sanders]

I made a change that removes the button entirely for 2ed or 3ed level nested levels as (as you said) it doesn't actually refer to a real workflow - but on reflection is that something we want?

Currently, the choice for whether to display that button looks at how many children nodes there are, and in the case there is only one, it will display (which works in cases where its looking at a single-valid-child), but Oliver has correctly pointed out that a single sub folder will be viewed the same way (although the folder is not in itself a workflow), which will obviously cause the question mark.

I think:

• If a node has only one child, collapse it [recursively].
• If the node was auto-collapsed, then use the workflow icon from its child for the collapsed node.
• If another child is subsequently added to the node auto-expand it.

E.G. this tree:

* a (workflow-part)
  * a1 (workflow) - running
* b (workflow-part)
  * b1 (workflow-part)
    * b11 (workflow) - running
* c (workflow-part)
  * c1 (workflow-part)
    * c11 (workflow) - running
  * c2 (workflow-part)
    * c22 (workflow) - running

Should be auto-collapsed as:

* a/a1 - running
* b/b1/b11 - running
* c/ - no-icon
  * c1/c11 - running
  * c2/c22 - running

With that logic the test for auto-collapsing would be something like:

function shouldAutoCollapse (node) {
    // return `false` if the node should not auto-collapse, return the tree's workflow status if it should
    const stack = [node]
    let item
    while (stack.length) {
        item = stack.pop()
        if (item.children.length === 1) {
           // there is only one child, this might be auto-collapsible, check children recursively to make sure
           stack.push(item.children[0])
        } else {
            // node has more than one children, this is not auto-collapsible
            return false
        }
    }
    return item.node.status
}

The test for displaying the workflow-icon on a workflow-part node would be:

shouldAutoCollapse(node) && !node.isExpanded

Untested!

[oliver-sanders]

Superseded by #1175?