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

scan: exclude Cylc 7 workflows from scan results #4771

Merged
merged 3 commits into from
Apr 3, 2022

Conversation

oliver-sanders
Copy link
Member

@oliver-sanders oliver-sanders commented Mar 25, 2022

An unintended side-effect of #4506 is that cylc scan is no longer able to tell between A Cylc 7 workflow that is running under Cylc 8 in compatibility mode and one that is running under Cylc 7.

This PR uses the log/workflow directory as a marker to exclude Cylc 7 workflows from scan results.

Note this fix also impacts the UIS which currently displays Cylc 7 workflows.

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.
  • Appropriate tests are included (unit and/or functional).
  • Appropriate change log entry included.
  • No documentation update required.

@oliver-sanders oliver-sanders added the bug Something is wrong :( label Mar 25, 2022
@oliver-sanders oliver-sanders added this to the cylc-8.0rc3 milestone Mar 25, 2022
@oliver-sanders oliver-sanders self-assigned this Mar 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.

Unfortunately, in my testing this merely makes a Cylc7 workflow that used to show up as myflow show up as myflow/log/suite now, when running cylc scan --state=all

CHANGES.md Outdated Show resolved Hide resolved
@oliver-sanders
Copy link
Member Author

oliver-sanders commented Mar 29, 2022

The log/workflow/log dir was tricking scan because it kinda looks like a flow itself.

Needed to change dir_is_flow to cover three cases:

  • It's a compatible workflow (show it in scan results).
  • It's an incompatible workflow (don't show it in scan results but also don't descend further looking for workflows).
  • It's not a workflow (descend further looking for workflows).

@@ -206,13 +233,14 @@ def _scan_subdirs(listing: List[Path], depth: int) -> None:
for task in done:
path, depth, contents = task.result()
running.remove(task)
if dir_is_flow(contents):
is_flow = dir_is_flow(contents)
Copy link
Member

@MetRonnie MetRonnie Mar 31, 2022

Choose a reason for hiding this comment

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

Might be worth repeating the type annotation here so that nobody removes the is_flow is False check below in an effort to tidy

Suggested change
is_flow = dir_is_flow(contents)
is_flow: Optional[bool] = dir_is_flow(contents)

tests/integration/test_scan.py Show resolved Hide resolved
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.

Checked out and tested.
I initially had a problem with this (a consequence of not tidying my run directories 🤦 ), I had a log directory from a previous run in the directory above my run dir which prevented it from showing. I suspect scan is going to need to be thought of when doing #4477, as this will (I think) potentially legitimize this scenario.

@hjoliver hjoliver merged commit 149bf3e into cylc:master Apr 3, 2022
@oliver-sanders oliver-sanders deleted the scan-cylc7 branch April 4, 2022 08:06
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
Development

Successfully merging this pull request may close these issues.

4 participants