10000 Handle stopped workflows (#493 follow-up) · Issue #494 · cylc/cylc-ui · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Handle stopped workflows (#493 follow-up) #494

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.

Sign up for GitHub

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

Jump to bottom
Closed
kinow opened this issue Sep 14, 2020 · 9 comments
Closed

Handle stopped workflows (#493 follow-up) #494

kinow opened this issue Sep 14, 2020 · 9 comments
Assignees
@kinow
Labels
bug Something isn't working
Milestone
0.6.0

Comments

@kinow
Copy link
Member
kinow commented Sep 14, 2020 •
edited
Loading

Describe the bug

Follow-up for #493. Now we have stopped workflows in the UI. But these workflows are not only stopped, they are stopped with their latest data.

So when a user navigates to a stopped workflow, we must display the last data. While also allowing for the subscription to restart (in case the user does a cylc run ... while looking at the UI).

Previously, we were clearing the tree with the data in the UI once the workflow stopped. That led to inconsistent states if the user refreshed the browser.

In this PR, we should address how the workflows are displayed

  • running workflow with data
  • stopped workflow without data (probably just a blank page and leave the rest for placeholders and empty states #332
  • stopped workflow with data
    • must give the same content if the user stops a running workflow, or if s/he opens a new tab for the workflow
    • must allow for the workflow to be restarted; this means that the current tree must then be cleared, so that we can start the deltas again from scratch (Note: we cannot simply restart the subscription, as the UI could be showing an old delta)

p.s.: also need to include the changelog for #493 that I missed while merging. (I've pushed a commit to include the missed changelog 👍 )

Release version(s) and/or repository branch(es) affected?

master

Steps to reproduce the bug

Experiment with stopped workflows. Start, stop it, restart it. Open a new tab, and compare with the lumino widget tree, or tree view.

Expected behavior

Screenshots

See #493

Additional context

See #493 for more about the issue.

Depends on cylc/cylc-uiserver#156

Related to cylc/cylc-flow#3813 (but I don't believe that's a requirement for this one 👍 )

Pull requests welcome!
This is an Open Source project - please consider contributing a bug fix
yourself (please read CONTRIBUTING.md before starting any work though).
@kinow kinow added the bug Something isn't working label Sep 14, 2020
@kinow kinow added this to the 0.3 milestone Sep 14, 2020
@kinow kinow self-assigned this Sep 14, 2020
@kinow
Copy link
Member Author
kinow commented Sep 14, 2020

Will start working on this one once the UIS PR is merged 👍

@kinow kinow changed the title Handle stopped workflows data (#493 follow-up) Handle stopped workflows (#493 follow-up) Sep 14, 2020
@hjoliver
Copy link
Member

@kinow is this about the treeview, not gscan?

@kinow
Copy link
Member Author
kinow commented Sep 15, 2020 •
edited
Loading

@kinow is this about the treeview, not gscan?

Yup, about the tree view. If you have Cylc 8 system running and then do something like

cylc run five
# open a tab with /user/hjoliver/#/workflows/five/
cylc stop five
# confirm the UI has recognized that the workflow stopped (e.g. GScan showing it greyed-out)
# open a new tab with /user/hjoliver/#/workflows/five/

At this point, the two tabs should be displaying the same, but at the moment they are not matching, due to how we are reacting to the last delta, and how we re-build the tree after deltas start arriving again (i.e. cylc run five again may create an even worse UI state in your 2 tabs)

@hjoliver
Copy link
Member

OK, what about the case where you close all tabs on the stopped flow, then go back to it later?

Then, the UI will forget the data (either immediately, or after 20s or whatever to allow for a possible quick return - but I don't think that's implemented yet), and go back to the UIS when the new tab is opened - is that right?

I mean we don't want to remember stopped flow state indefinitely, if there are no tabs open.

@kinow
Copy link
Member Author
kinow commented Sep 15, 2020

OK, what about the case where you close all tabs on the stopped flow, then go back to it later?

In this case, the data is destroyed when the tabs are closed. Going back to it will start a new deltas subscription.

Then, the UI will forget the data (either immediately, or after 20s or whatever to allow for a possible quick return - but I don't think that's implemented yet), and go back to the UIS when the new tab is opened - is that right?

Yeah, not implemented yet. You are correct it starts a new WebSocket + 1 long request to handle the initial data burst and build the tree.

I mean we don't want to remember stopped flow state indefinitely, if there are no tabs open.

+1 the only case I think is incorrect, is when you have the workflow running, then stop it. At this point your workflow view is inconsistent if you open one more tab.

@hjoliver
Copy link
Member

Thanks, all good then 👍

@oliver-sanders oliver-sanders modified the milestones: 0.3, 0.4 Mar 4, 2021
@oliver-sanders oliver-sanders modified the milestones: 0.4.0, 0.5.0 Apr 16, 2021
@kinow
Copy link
Member Author
kinow commented Jun 24, 2021

See #556 too

@kinow
Copy link
Member Author
kinow commented Jun 24, 2021

+1 the only case I think is incorrect, is when you have the workflow running, then stop it. At this point your workflow view is inconsistent if you open one more tab.

Note to self, I think this diff (against GScan branch) would fix the issue described in the paragraph above:

diff --git a/src/components/cylc/tree/deltas.js b/src/components/cylc/tree/deltas.js
index e51a0d64..66cd4be9 100644
--- a/src/components/cylc/tree/deltas.js
+++ b/src/components/cylc/tree/deltas.js
@@ -215,7 +215,7 @@ export default function (data, workflow, lookup, options) {
   const deltas = data.deltas
   // first we check whether it is a shutdown response
   if (deltas.shutdown) {
-    CylcTree.clear(workflow)
+    // CylcTree.clear(workflow)
     return {
       errors: []
     }

Simply removing that call should be enough, to keep the data in the UI. That, combined with #556, would give a much better experience for users of the tree view/component. Will look at these two issues after the GScan deltas branch is reviewed/merged.

@oliver-sanders oliver-sanders modified the milestones: 0.5.0, 0.6.0 Jul 16, 2021
@kinow
Copy link
Member Author
kinow commented Oct 6, 2021 •
edited
Loading

This was fixed by #700 🎉

image

(stopped five, the state in the UI hasn't changed, can navigate to another workflow, and go back, state is the same)

@kinow kinow closed this as completed Oct 6, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@kinow kinow

Labels
bug Something isn't working
Projects
None yet
Milestone
0.6.0
Development

No branches or pull requests
3 participants
@kinow @hjoliver @oliver-sanders
0