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
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions CHANGES.md
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,9 @@ ones in. -->
[#4777](https://github.com/cylc/cylc-flow/pull/4777) -
Reinstate the Cylc 7 template variables for xtriggers with deprecation warnings.

[#4771](https://github.com/cylc/cylc-flow/pull/4771) -
Fix issue where Cylc 7 workflows could show in `cylc scan` output and in the UI.

[#4720](https://github.com/cylc/cylc-flow/pull/4720) - Fix traceback in
workflow logs when starting or reloading a workflow with an illegal item
(e.g. typo) in the config.
Expand Down
42 changes: 35 additions & 7 deletions cylc/flow/network/scan.py
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,6 @@
FLOW_FILES = {
# marker files/dirs which we use to determine if something is a flow
WorkflowFiles.Service.DIRNAME,
WorkflowFiles.SUITE_RC, # cylc7 flow definition file name
WorkflowFiles.FLOW_FILE, # cylc8 flow definition file name
WorkflowFiles.LOG_DIR
}
Expand All @@ -94,20 +93,48 @@
}


def dir_is_flow(listing: Iterable[Path]) -> bool:
def dir_is_flow(listing: Iterable[Path]) -> Optional[bool]:
"""Return True if a Path contains a flow at the top level.

Args:
listing (list):
listing:
A listing of the directory in question as a list of
``pathlib.Path`` objects.

Returns:
bool - True if the listing indicates that this is a flow directory.
- True if the directory:
- Is a Cylc 8 workflow.
- Is a Cylc 7 workflow that has not yet been run.
- Is a Cylc 7 workflow running under Cylc 8 in compatibility mode.
- False if the directory:
- Is not a workflow.
- None if the directory:
- Is an incompatible workflow (e.g. a Cylc 7 workflow running under
Cylc 7).

"""
names = {path.name for path in listing}
return bool(FLOW_FILES & names)

if WorkflowFiles.SUITE_RC in names:
# a Cylc 7 workflow ...
for path in listing:
if path.name == WorkflowFiles.LOG_DIR:
if (path / 'workflow').exists():
# Cylc 8: log/workflow/log
return True
else:
# Cylc 7: log/suite/log
return None
# workflow doesn't have a log dir so has not been run
# so could be either a Cylc 7 or a Cylc 8 workflow
return True

if FLOW_FILES & names:
# a pure Cylc 8 workflow
return True

# random directory
return False


@pipe
Expand Down Expand Up @@ -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)

if is_flow:
# this is a flow directory
yield {
'name': str(path.relative_to(run_dir)),
'path': path,
}
elif depth < max_depth:
elif is_flow is False and depth < max_depth:
# we may have a nested flow, lets see...
_scan_subdirs(contents, depth)
# don't allow this to become blocking
Expand Down
51 changes: 51 additions & 0 deletions tests/integration/test_scan.py
Original file line number Diff line number Diff line change
Expand Up @@ -406,3 +406,54 @@ async def test_scan_sigstop(
(30, f'Workflow not running: {name}')
in [(level, msg) for _, level, msg in caplog.record_tuples]
)


@pytest.fixture
def cylc7_run_dir(tmp_path):
"""A run directory containing three Cylc 7 workflows."""
# a workflow that has not yet been run
# (could be run by either cylc 7 or 8 so should appear in scan results)
either = tmp_path / 'either'
either.mkdir()
(either / WorkflowFiles.SUITE_RC).touch()

# a Cylc 7 workflow that has been / is being run by Cylc 7
# (should not appear in scan results)
cylc7 = tmp_path / 'cylc7'
cylc7.mkdir()
(cylc7 / WorkflowFiles.SUITE_RC).touch()
Path(cylc7, WorkflowFiles.LOG_DIR, 'suite').mkdir(parents=True)
Path(cylc7, WorkflowFiles.LOG_DIR, 'suite', 'log').touch()

# a Cylc 7 workflow running under Cylc 8 in compatibility mode
# (should appear in scan results)
cylc8 = tmp_path / 'cylc8'
cylc8.mkdir()
(cylc8 / WorkflowFiles.SUITE_RC).touch()
Path(cylc8, WorkflowFiles.LOG_DIR, 'workflow').mkdir(parents=True)
Path(cylc8, WorkflowFiles.LOG_DIR, 'workflow', 'log').touch()
oliver-sanders marked this conversation as resolved.
Show resolved Hide resolved

# crazy niche case of a Cylc 7 workflow that has had its DB removed
# and re-run under Cylc 8
# (should appear in scan results)
cylc8 = tmp_path / 'cylc78'
cylc8.mkdir()
(cylc8 / WorkflowFiles.SUITE_RC).touch()
Path(cylc8, WorkflowFiles.LOG_DIR, 'suite').mkdir(parents=True)
Path(cylc8, WorkflowFiles.LOG_DIR, 'suite', 'log').touch()
Path(cylc8, WorkflowFiles.LOG_DIR, 'workflow').mkdir(parents=True)
Path(cylc8, WorkflowFiles.LOG_DIR, 'workflow', 'log').touch()

return tmp_path


async def test_scan_cylc7(cylc7_run_dir):
"""It should exclude Cylc 7 workflows from scan results.

Unless they are running under Cylc 8 in Cylc 7 compatibility mode.
"""
assert await listify(
scan(cylc7_run_dir)
) == [
'cylc78', 'cylc8', 'either'
]
File renamed without changes.