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 icons - no icons for held and retrying? #189

Closed
kinow opened this issue Aug 15, 2019 · 22 comments
Closed

task icons - no icons for held and retrying? #189

kinow opened this issue Aug 15, 2019 · 22 comments
Milestone

Comments

@kinow
Copy link
Member

kinow commented Aug 15, 2019

Hi,

I am using the dummy data for the WorkflowService for the #145 issue, same data feeding the GScan/Tree when mode=offline right now.

There are two tasks that don't get any icons when rendered with the Task component. One task is named sleepy and has the state=held, and the other is named retrying, with state=retrying.

I just had a look at the Task.vue, and it doesn't seem to have CSS for these values yet... is it planned for the future, or should I use the component in some different way when I have a task with these states?

Thanks!

image

@kinow kinow mentioned this issue Aug 20, 2019
@hjoliver
Copy link
Member

I just had a look at the Task.vue, and it doesn't seem to have CSS for these values yet...

This is presumably deliberate by @oliver-sanders - see this conversation:

cylc/cylc-admin#47 (comment)

"held" is no longer a task state - it's an attribute of a task with whatever state.

"retry" will become something similar, but we haven't finalized how that will work yet.

@kinow
Copy link
Member Author

kinow commented Aug 20, 2019

Oh, thanks for the link and explanation @hjoliver . I guess then the component will be updated later to have something like a boolean props (flag/parameter) saying whether that Task is held, retry, etc. Should we close this ticket then?

@dwsutherland
Copy link
Member

dwsutherland commented Aug 20, 2019

I guess then the component will be updated later to have something like a boolean props (flag/parameter) saying whether that Task is held, retry, etc. Should we close this ticket then?

That's with/after;
cylc/cylc-flow#3278
which should go in after:
cylc/cylc-flow#3202

The UI Server will/should get it automatically

@oliver-sanders
Copy link
Member

This is presumably deliberate by @oliver-sanders - see this conversation:

So yeah this is deliberate.

To sum up, here are the currently unsupported task statuses:

@kinow
Copy link
Member Author

kinow commented Aug 20, 2019

Thanks for the explanation @oliver-sanders ! Looks like it will take some time until we get everything ready to send the right statuses from the server, and to properly display them in the UI.

@hjoliver
Copy link
Member

I have an alternative idea for the ready state: #194

@hjoliver
Copy link
Member

I mostly agree with @oliver-sanders above, except for:

  • Ready: should be displayed as queued rather than waiting - see task status icon tweak proposal #194

  • Runahead: not sure about this.

    • it matters to users who have reason to change the default runahead limit
    • and while we still have a task pool, users need to know about the runahead pool if they insert tasks beyond the runahead limit (which is not uncommon) ... if they can't see the runahead pool they will think their insertion attempt failed.
    • (I agree that it would be nice to hide this from users, but I think the above two use cases mean we can't do that ... until we implement spawn-on-demand).

@hjoliver
Copy link
Member

hjoliver commented Aug 20, 2019

@kinow - would it be easy for you to put in a default icon (and/or the status name) to indicate where we still have missing icons? (or status values that we will be getting rid of later on)

@kinow
Copy link
Member Author

kinow commented Aug 20, 2019

Oh, good idea. Adding slots to the Tree component will demans a lot of time and concentration, and I was planning on taking a break to submit PR's for the borderless/flat divs, workflow play/stop/pause icons, etc.

Will prepare one to show an icon that means the status was not found, and log to browser console too.

@kinow
Copy link
Member Author

kinow commented Aug 21, 2019

@hjoliver #200

@dwsutherland
Copy link
Member

I guess I should add runahead elements to the data store then 😅

@hjoliver
Copy link
Member

Not yet - we're still trying to figure out if we can get away without runahead tasks in the UI.

@oliver-sanders
Copy link
Member

oliver-sanders commented Aug 21, 2019

Runahead: not sure about this.

Fair enough, there are definitely cases where users require knowledge of the runahead pool, I was hoping we could find alternative representation of this information which don't involve populating the UI with the runahead pool.

Not yet - we're still trying to figure out if we can get away without runahead tasks in the UI.

Cases where the user requires information about the runahead pool:

  • assessing the runahead limit / max active cycles
    • User needs to know if a suite is approaching the limit.
    • User doesn't need to actually see the runahead tasks.
    • Could be represented in text e.g:
      • 3 of 3 active cycles currently running.
      • current runahead is P1D6H, the maximum is P3D.
  • Insert beyond runahead limit
    • If a task is inserted in the runahead pool the user won't be able to see it
    • They won't be able to do anything with it until it enters the proper pool anyway will they?
    • So really we just need to warn the user that this is going to happen.
    • Perhaps with a quick check similar to the --no-warn option?
  • cylc remove --no-spawn
    • User needs to know that a task is not in the runahead pool???
  • Others?

@hjoliver
Copy link
Member

Fair enough ... I've come around. Let's not expose the runahead pool in the UI.

Instead we could:

  • rely on documentation and warnings
  • maybe disallow insertion beyond the runahead limit, unless forced?
  • maybe provide a command to specifically query the runahead pool, mainly for use by developers and expert users?

@dwsutherland
Copy link
Member

We report the closest run ahead cycle, and the state total (although the latter needs fixed).. So if tasks are inserted past the run ahead, won’t this be enough? Given they can’t act on them anyway

Sometimes, it’s desirable to check if a task has spawned (monthly tasks in a daily cycling workflow)... But aren’t we trying to hide that behaviour?

Again wouldn’t be hard to add the edges and “ghost nodes” of the runahead cycles (it’s just static data until it changes pool).. But the view might not be commensurate with the N-edges view (or might it?)

@hjoliver
Copy link
Member

@dwsutherland - yes, I think that will be enough; and yes, we are trying to hide that behaviour (with the hybrid N-edges view, at least).

@wxtim
Copy link
Member

wxtim commented Sep 12, 2019

@kinow Can we wrap this up? Or assign it some sort of info-only label?

@kinow
Copy link
Member Author

kinow commented Sep 12, 2019

@wxtim I'm actually quite out of the loop here. There is a new icon now, with a question mark in the middle of the circle. Plus, whenever an unknown state appears, we log it to the browser console.

But I think the discussion above, amongst @hjoliver, @dwsutherland , and @oliver-sanders, is a bit broader than what I expected for this issue. I am not sure if we can close this issue, and then create follow up tickets for the held/retrying/etc, or if there is more to discuss here.

I'm happy to have an icon on the UI, so good enough for more for now 😬 so +1 for closing from me.

@hjoliver
Copy link
Member

hjoliver commented Sep 13, 2019

I'll review this issue and close it soon, when I get some time. @kinow is right that the main topic was addressed (there's now a ? icon for unknown state, which should never be needed once we've settled on the final icon and state set). But I'd like to make sure we've captured all the other points discussed before closing...

@kinow kinow added this to the 0.2 milestone Sep 14, 2019
@kinow
Copy link
Member Author

kinow commented Nov 11, 2019

I'll review this issue and close it soon, when I get some time. @kinow is right that the main topic was addressed (there's now a ? icon for unknown state, which should never be needed once we've settled on the final icon and state set). But I'd like to make sure we've captured all the other points discussed before closing...

I think we have have the workaround for task icons, but missing for job icons now too. Should we create a separate issue?

@oliver-sanders
Copy link
Member

I think the only job status we now don't support is "ready" (which will soon become "preparing"). I think we want to make this a task-only state anyway (since the job doesn't exist / hasn't been submitted yet) so this state should disappear.

@kinow
Copy link
Member Author

kinow commented Nov 12, 2019

Thanks @oliver-sanders ! No need for another issue then. Then I guess we can close this issue now.

@kinow kinow closed this as completed Nov 12, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants