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

tui: bug fixes #4670

Merged
merged 5 commits into from
Feb 25, 2022
Merged

tui: bug fixes #4670

merged 5 commits into from
Feb 25, 2022

Conversation

oliver-sanders
Copy link
Member

tui: add offline commands

  • Add support for the play and clean commands.

zmq client: raise WorkflowStoped not ClientTimeout

tui: disable mutations for stopped workflows

tui: fix context menu for cycle points

Note: This PR includes a sneaky enhancement (offline) which shouldn't really be going into a
RC release.
It's small enough and Tui as an experimental interface is supplementary enough I think its ok, but happy to cherry-pick and delay to 8.0.1.

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).
  • Applied any dependency changes to both setup.cfg and conda-environment.yml.
  • Tui has no test framework for live testing at the moment
  • Appropriate change log entry included.
  • No documentation update required.

@oliver-sanders oliver-sanders added the bug Something is wrong :( label Feb 9, 2022
@oliver-sanders oliver-sanders added this to the cylc-8.0rc2 milestone Feb 9, 2022
@oliver-sanders oliver-sanders self-assigned this Feb 9, 2022
@oliver-sanders oliver-sanders force-pushed the tui-fixes branch 2 times, most recently from 0c19ab8 to 9777156 Compare February 10, 2022 10:50
* Closes cylc#4629.
* Also adds the node ID to the context menu.
* WorkflowClient could raise ClientTimeout for stopped workflows.
* Closes cylc#4654
* Add support for the play and clean commands.
@MetRonnie
Copy link
Member

Poking tests

@MetRonnie MetRonnie closed this Feb 24, 2022
@MetRonnie MetRonnie reopened this Feb 24, 2022
Copy link
Member

@MetRonnie MetRonnie left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested out manually, fixes the reported bugs.

Only thing is there is a delay of a couple of seconds between hitting enter on one of the offline commands in the menu and the menu closing

@oliver-sanders
Copy link
Member Author

That's the time it takes for the command to run (blocking).

Copy link
Member

@wxtim wxtim left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Can confirm that this definitely fixes #4654 , #4647 and #4629.

I'm concerned about the bug shown below, although it might want spinning into a new bug report:

output.mp4

Essentially - stop returns control to you before the mutation finishes, and you can bring back the menu and ask for stop a second time after the workflow has stopped, and this causes a crash with traceback. Mentioned here rather than a separate issue because traceback refers to Exception raised by code in this PR.

@oliver-sanders your choice whether you want to fix or log this bug. I'm otherwise happy enough that this answers its original purpose that it can go in.

@oliver-sanders
Copy link
Member Author

^ there

@wxtim wxtim self-requested a review February 25, 2022 14:31
@wxtim wxtim merged commit 52ab195 into cylc:master Feb 25, 2022
Copy link
Member

@MetRonnie MetRonnie left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cannot reproduce the bug Tim identified with the latest commit.

(Oh, it's already been merged!)

@oliver-sanders oliver-sanders deleted the tui-fixes branch February 25, 2022 14:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something is wrong :(
Projects
None yet
3 participants