Cylc cli runN fix #4228
Cylc cli runN fix #4228
Conversation
|
Good spot @datamel, I hadn't noticed this bug. |
cylc/flow/terminal.py
Outdated
| r'(?:run)(\d*$)', | ||
| os.readlink(workflow_dir)).group(1) | ||
| # importing here to counter circular dependancy errors. | ||
| from cylc.flow.workflow_files import WorkflowFiles |
There was a problem hiding this comment.
Oh really, workflow_files imports from here? or is it more indirect?
There was a problem hiding this comment.
Yes, more indirect. Circle of dependencies can be seen pulling the import to the top... something like scripts/__init__ => terminal => workflow_files => remote => options_parser => terminal .
I'm just pushing a change to fix this, rather than leaving the import at time of use.
cylc/flow/terminal.py
Outdated
| workflow_dir = get_workflow_run_dir(arg) | ||
| else: | ||
| workflow_dir = arg | ||
| run_number = re.search( #type: ignore |
There was a problem hiding this comment.
I think this is ok ignoring this mypy...
Unless I am missing something, as far as I can see, mypy thinks that re.search could return none, and therefore can not have group(1) applied. This is called on condition that the arg ends in runN (or runN/) and so should never return none when the symlink is followed.
There was a problem hiding this comment.
Shouldn't happen but is technically possible, could add an if statement to handle this case.
| endings = (WorkflowFiles.RUN_N, f'{WorkflowFiles.RUN_N}/') | ||
| args = [ | ||
| parse_reg(cli_arg) if cli_arg.endswith(endings) | ||
| else cli_arg for cli_arg in args | ||
| ] |
There was a problem hiding this comment.
I'm not very keen on doing this to CLI args arbitrarily, however, the parsing of the registration argument is not properly centralised at the moment so there isn't really anywhere better to put it.
Adding a note to #4230 (comment) as we should be getting onto that shortly so can leave until then.
This fixes a bug whereby if using runN on the cli, the flow name is not correctly mapped to the actual run number.
To reproduce the bug..
Have an installed workflow, myflow/runN which maps to e.g. myflow/run1
start a workflow off
cylc play myflow/runNstop the workflow
cylc stop myflow/run1 --now --nowa
cylc scanshould confirm the workflow is still running - since the workflow was started withmyflow/runN, this is used rather thanmyflow/run1as the registered workflow name.This is a small change with no associated Issue.
CONTRIBUTING.mdand added my name as a Code Contributor.setup.pyandconda-environment.yml.