-
Notifications
You must be signed in to change notification settings - Fork 93
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
cylc monitor rewrite #3463
cylc monitor rewrite #3463
Conversation
Need to add urwid to |
Found a couple of issues in my environment already, but it's a WIP; generally: god damn it, that is cool. Well done. 🎉 |
863fa0f
to
81e9913
Compare
The test I added for the new Have added a TODO for performance, there might be some easy wins. If nothing else, we can lower the refresh rate for heavy suites. |
81e9913
to
befb73a
Compare
@oliver-sanders - I'm keen to get this in, and we can enhance it further (e.g. status summaries at top) in future PRs. However, (CylcCon 2020) @trwhitcomb wanted to keep the old What about |
Things I'll try and do in this PR:
Things I'll try and punt to future PRs:
I think once filtering/state-totals/copyable text are done the use case for the old
With the addition of a gscan column this program becomes a CLI equivalent to the Cylc WUI. Perhaps:
|
Great.
Yep, good.
I tend to agree, but we can treat that as a separate issue since the two utils won't have the same name anymore.
Ah, good point. The name might require some thought. |
Two approvals, if Travis passes with no issues I think it should be ready to be merged. Thanks @oliver-sanders ! |
One failure, but doesn't look related. I don't seem to have access to restart the build (button not displaying, maybe a Travis glitch). Doing some manual testing... |
Found another weird case with The terminal at the left top corner has the workflow I did a However, Did a quick Then after hitting CTRL + C, it printed an exception and the process started working again. |
Hard to reproduce, if you get it again could you see if the suite is still responsive, try a graphql request. I'm going to keep Tui open on a suite in the background and see if it freezes at any point. |
It was running, at least the logs were showing the tasks/jobs and new cyclepoints as it normally does. After clicking CTRL + C on the Will merge & create an issue for that tomorrow morning so we can track the issue 👍 thanks! |
Would rather fix now but I'm finding it very difficult to reproduce and harder to debug. (I'm getting other issues on my platform at the moment which are hard to separate). |
Managed to reproduce, the traceback is coming from I think what's going on is that something has gone wrong with the connection so Tui has attempted to create a new connection. This has caused Investigating... |
Not sure if related, but these are the issues in flow/uiserver I created when working on Cylc UI and that have some ZMQ stuff in the tracebacks: |
If you confirm that as the cause, maybe for the moment we should not attempt to automatically reconnect? (Or is it possible to do it in a way that is more like starting the application again from scratch)? |
@oliver-sanders appears to be some consistent failures in Travis chunk3:
|
Should be fixed now. |
Tests pass, two approvals, and this is quite self-contained ... going for the merge 🎉 |
Synced local branch, then tested again with Randomly held then released workflows. The first two times, it worked fine for Then on the third time the workflow exited, but that's a known issue. Started it again with
Better to merge it and move it to a separate issue if others have a similar problem (might be something I am doing). |
Must admit I didn't test hold/release extensively.
|
Testing with a simpler suite:
It appears that the hold indicator does not get applied and/or removed to complete cycles of finished tasks. (For which it is kind of meaningless anyway ... but probably easy for @oliver-sanders to fix). |
Yep, confirmed. The tasks which appear to be out-of-sync should not be visible and are somehow left-over from the previous update. If you run |
After a bit of digging the issue is not here (you can replicate the same in WUI), Tui is simply displaying the data it was provided. This is an artifact of the two task pools we currently have in Cylc, the SOS |
Thanks for investigating it @oliver-sanders . I guess that might be something being fixed. @dwsutherland mentioned he has been finding small issues in the data store while working on the UIS deltas (if I understand that correctly). |
Ah yes, makes sense, I should have realized that. I talked to @dwsutherland about this a while ago Not his fault, there's some ambiguity about what exactly to provide to the UI for the default "view" until one or other of the following is settled on and solved:
For work on SOD I really need to see just the scheduler task pool (so was using the old monitor which still feed off of the old scheduler data store). |
This isn't a bug, it's absolutely correct, when you hold a suite you are only holding active tasks, not archived ones.
|
What’s not a bug? Showing the hold indicator on “archived” tasks, or displaying those archived tasks at all in the current UI? I would say the former is a bug (as you say, you can’t hold an archived task), and the latter isn’t a bug but is sort of wrong in that it doesn’t conform to our current SOS task pool or any of the future options as I listed above. |
When you hold a suite only the "active" tasks actually get held and so show up that way in the GUI. Technically this is the correct behaviour, the "archived" tasks are not held, the database backs this up. |
I agree with that, but I thought (maybe I was wrong?) that the hold indicator was being applied to archived tasks in |
Looks like tasks keep the status they had when they were in the task pool. Subsequent holds, etc do not effect "archived" tasks. |
Closes #1685
Jet-lag washup - re-implement
cylc monitor
usingurwid
.cylc monitor
.ctrl +
.What's supported:
What's not supported:
ctrl c
exit, useq
orctrl d
.TODO
cylc monitor
.Investigate performance with larger suites.attempt after delta updatesRequirements check-list
CONTRIBUTING.md
and added my name as a Code Contributor.