tui: add vr #5381
tui: add vr #5381
Conversation
oliver-sanders
force-pushed
the
tui-vr
branch
from
dd756b9
to
fbe6b87
Compare
oliver-sanders
force-pushed
the
tui-vr
branch
from
fbe6b87
to
14fb785
Compare
|
(bandit a little unhappy) |
|
(codecov a lottle unhappy) |
There was a problem hiding this comment.
- Code seems sensible.
- Tested locally as working.
- Confirm all tests are failing due to CI issues - Not true!
- All functional tests passing (failure is on codecov)
I'd observe that VR seems pretty slow, especially to play, _after the task runnin has been closed. I was wondering if we could parse the stderr of the subprocess as it's running and announce when each step is done.
??? |
Only as slow as
|
We could stream stdout/err for all commands but that's a big job for another day! |
I spent a few minutes looking and came to this conclusion. I'm jotting it on my training day stuff. |
|
Those integration test failures don't make a lick of sense, closing / re-opening on the wild offchange that it makes a difference. |
|
I've repro'd the fails locally in a clean environment. :( |
|
I've got tests passing with Python 3.7 and failing with 3.9 locally, yikes! |
|
Ach, gottit, the |
This fixes it for me - still needs a deconflict. Haven't approved so that you can poke me and I can press merge once you've sorted it. |
I think that's right correct behaviour right, it allows the user to read the error message? |
| ) | ||
| out, err = proc.communicate() | ||
| if proc.returncode != 0: | ||
| raise ClientError(f'Error in command {" ".join(cmd)}\n{err}') |
There was a problem hiding this comment.
What do you think of
| raise ClientError(f'Error in command {" ".join(cmd)}\n{err}') | |
| raise ClientError(f'Error in command {" ".join(cmd)}\n{err or out}') |
There was a problem hiding this comment.
I'm not sure the output viewer is up to scratch for displaying the full output (small narrow section of the window).
|
Wait a minute, doesn't this need to be included in 8.1.x? Edit: actually just the bit about the empty error box |
Response to user feedback:
--yesoption tocylc reinstall/cylc vr.cylc vrintocylc tui.This also presents a box which tells the user that a command is running to avoid confusion with longer-running commands e.g.
play,clean,validate-reinstall.Check List
CONTRIBUTING.mdand added my name as a Code Contributor.setup.cfg(andconda-environment.ymlif present).cylc tuiat present.CHANGES.mdentry included if this is a change that can affect users?.?.xbranch.