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

Disallow run n and run x as flow names #4526

Merged
merged 7 commits into from
Nov 29, 2021

Conversation

wxtim
Copy link
Member

@wxtim wxtim commented Nov 22, 2021

closes #4394 - IMO a single reviewer PR.

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.py and
    conda-environment.yml.
  • Appropriate tests are included (unit and/or functional).
  • Appropriate change log entry included.
  • (master branch) I have opened a documentation PR at Disallow run n and run x as flow names cylc-doc#327.

@wxtim wxtim self-assigned this Nov 22, 2021
@wxtim wxtim changed the title prevent cylc install being OK with runN and run\d+ folder names Disallow run n and run x as flow names Nov 22, 2021
@wxtim wxtim added this to the cylc-8.0rc1 milestone Nov 22, 2021
@wxtim wxtim requested review from MetRonnie and datamel and removed request for datamel November 22, 2021 15:32
@wxtim wxtim force-pushed the disallow_runN_and_runX_as_flow-names branch from b1750d0 to 3690653 Compare November 24, 2021 07:46
cylc/flow/workflow_files.py Outdated Show resolved Hide resolved
@wxtim wxtim force-pushed the disallow_runN_and_runX_as_flow-names branch from 234be0f to 9123245 Compare November 24, 2021 08:09
@wxtim wxtim requested a review from MetRonnie November 24, 2021 08:10
@@ -1311,7 +1311,7 @@ def _parse_src_reg(reg: Path, cur_dir_only: bool = False) -> Tuple[Path, Path]:
return (reg, abs_path)


def validate_workflow_name(name: str) -> None:
def validate_workflow_name(name: str, runNcheck=False) -> None:
Copy link
Member

Choose a reason for hiding this comment

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

Why do we need this runNcheck argument?

Copy link
Member Author

@wxtim wxtim Nov 25, 2021

Choose a reason for hiding this comment

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

Other commands that use validate_workflow_name can (sometimes should) be passed a name which ends in runN or runX:

Consider cylc clean /home/me/cylc-run/my-workflow/runN. Cylc clean uses validate workflow, and in that case it certainly doesn't want to claim that /home/me/cylc-run/my-workflow/runN is not valid.

Cylc play is the other use case.

cylc/flow/workflow_files.py Outdated Show resolved Hide resolved
cylc/flow/workflow_files.py Outdated Show resolved Hide resolved
Co-authored-by: Ronnie Dutta <61982285+MetRonnie@users.noreply.github.com>
Copy link
Contributor

@datamel datamel left a comment

Choose a reason for hiding this comment

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

Thanks @wxtim. I have read and checked out the code and run the tests.

@datamel datamel merged commit 9652260 into cylc:master Nov 29, 2021
@wxtim wxtim deleted the disallow_runN_and_runX_as_flow-names branch November 29, 2021 13:47
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

Successfully merging this pull request may close these issues.

Disallow runX as a workflow name
3 participants