From 1dd682998e3ce2da29015b456df2706d33c44e3e Mon Sep 17 00:00:00 2001 From: Oliver Sanders Date: Mon, 1 Aug 2022 13:32:38 +0100 Subject: [PATCH 01/66] queues: set default limit * Closes #5010 * Set `[scheduling][queues][default]limit=100`. --- cylc/flow/cfgspec/workflow.py | 2 +- tests/functional/cylc-config/00-simple/section1.stdout | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/cylc/flow/cfgspec/workflow.py b/cylc/flow/cfgspec/workflow.py index 205e588decd..f242adb0c9a 100644 --- a/cylc/flow/cfgspec/workflow.py +++ b/cylc/flow/cfgspec/workflow.py @@ -691,7 +691,7 @@ def get_script_common_text(this: str, example: Optional[str] = None): with Conf('default', meta=Queue, desc=''' The default queue for all tasks not assigned to other queues. '''): - Conf('limit', VDR.V_INTEGER, 0, desc=''' + Conf('limit', VDR.V_INTEGER, 100, desc=''' Controls the total number of active tasks in the default queue. diff --git a/tests/functional/cylc-config/00-simple/section1.stdout b/tests/functional/cylc-config/00-simple/section1.stdout index 1e60552fe19..972b7a55606 100644 --- a/tests/functional/cylc-config/00-simple/section1.stdout +++ b/tests/functional/cylc-config/00-simple/section1.stdout @@ -8,7 +8,7 @@ cycling mode = integer runahead limit = P4 [[queues]] [[[default]]] - limit = 0 + limit = 100 members = [[special tasks]] clock-trigger = From 42062bb88886d010d9df8bb614689bf60f2d0847 Mon Sep 17 00:00:00 2001 From: Hilary James Oliver Date: Tue, 2 Aug 2022 16:27:20 +1200 Subject: [PATCH 02/66] Update change log. --- CHANGES.md | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/CHANGES.md b/CHANGES.md index 5e6a0bebf75..509ec27aebc 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -28,6 +28,16 @@ creating a new release entry be sure to copy & paste the span tag with the `actions:bind` attribute, which is used by a regex to find the text to be updated. Only the first match gets replaced, so it's fine to leave the old ones in. --> + +------------------------------------------------------------------------------- +## __cylc-8.1.0 (Upcoming)__ + +### Enhancements + +[#5032](https://github.com/cylc/cylc-flow/pull/5032) - set a default limit of +100 for the "default" queue. + + ------------------------------------------------------------------------------- ## __cylc-8.0.0 (Released 2022-07-28)__ From 8fce5c77a498def2668b5c5a7079506fd6fd1967 Mon Sep 17 00:00:00 2001 From: Oliver Sanders Date: Tue, 2 Aug 2022 10:51:47 +0100 Subject: [PATCH 03/66] Update README.md --- README.md | 62 +++++++++++++++++++++++++++---------------------------- 1 file changed, 31 insertions(+), 31 deletions(-) diff --git a/README.md b/README.md index 76890b6769f..ce4207cd47c 100644 --- a/README.md +++ b/README.md @@ -20,42 +20,42 @@ that specialises in cycling workflows and has strong scaling characteristics. Cylc was originally developed to meet the challenges of production weather forecasting - which is notorious for the size and complexity of its workflows. -### Citations & Publications - -[![DOI](https://zenodo.org/badge/1836229.svg)](https://zenodo.org/badge/latestdoi/1836229) -[![JOSS](http://joss.theoj.org/papers/10.21105/joss.00737/status.svg)](https://doi.org/10.21105/joss.00737) -[![CISE](https://img.shields.io/website/https/ieeexplore.ieee.org/document/8675433.svg?color=orange&label=CISE&up_message=10.1109%2FMCSE.2019.2906593)](https://ieeexplore.ieee.org/document/8675433) +### Quick Start -### Cylc 7 (legacy) -![python](https://img.shields.io/badge/python-2.6%20%7C%202.7-orange) -[![Documentation](https://img.shields.io/website?label=documentation&up_message=live&url=https%3A%2F%2Fcylc.github.io%2Fcylc-doc%2F7.9.3%2Fhtml%2Findex.html)](https://cylc.github.io/cylc-doc/7.9.3/html/index.html) - -* HTTPS network layer. -* PyGTK GUI. -* On the `7.8.x` branch in the source code. -* 7.8 - Python 2.6 -* 7.9 - Python 2.7 - -[Installation](https://github.com/cylc/cylc-flow/blob/7.8.x/INSTALL.md) | -[Documentation](https://cylc.github.io/documentation) - -### Cylc 8 (production) - -![PyPI](https://img.shields.io/pypi/pyversions/cylc-flow.svg?color=green) -[![PyPI](https://img.shields.io/pypi/v/cylc-flow.svg?color=yellow)](https://pypi.org/project/cylc-flow/) -[![Anaconda-Server Badge](https://anaconda.org/conda-forge/cylc-flow/badges/version.svg)](https://anaconda.org/conda-forge/cylc-flow) -[![Documentation](https://img.shields.io/website?label=documentation&up_message=live&url=https%3A%2F%2Fcylc.github.io%2Fcylc-doc%2Fstable%2Fhtml%2Findex.html)](https://cylc.github.io/cylc-doc/latest/html/index.html) +[Installation](https://cylc.github.io/cylc-doc/stable/html/installation.html) | +[Documentation](https://cylc.github.io/cylc-doc/stable/html/index.html) -* ZMQ (TCP) network layer. -* Text-based terminal user interface (TUI). -* Optional web-based graphical user interface (GUI) provided by ([cylc-uiserver](https://github.com/cylc/cylc-uiserver)). -* On the `master` branch in the source code. +```bash +# install cylc +conda install cylc-flow + +# write your first workflow +mkdir -p ~/cylc-src/example +cat > ~/cylc-src/example/flow.cylc <<__CONFIG__ +[scheduling] + initial cycle point = 1 + cycling mode = integer + [[graph]] + P1 = a => b[-P1] => c & d +[runtime] + [[a, b, c, d]] + script = echo "Hello $CYLC_TASK_NAME" +__CONFIG__ + +# install and run it +cylc install example +cylc play example + +# watch it run +cylc tui example +``` -Cylc 8 is now production-ready. +### Citations & Publications -[Installation](https://cylc.github.io/cylc-doc/stable/html/installation.html) | -[Documentation](https://cylc.github.io/cylc-doc/stable/html/index.html) +[![DOI](https://zenodo.org/badge/1836229.svg)](https://zenodo.org/badge/latestdoi/1836229) +[![JOSS](http://joss.theoj.org/papers/10.21105/joss.00737/status.svg)](https://doi.org/10.21105/joss.00737) +[![CISE](https://img.shields.io/website/https/ieeexplore.ieee.org/document/8675433.svg?color=orange&label=CISE&up_message=10.1109%2FMCSE.2019.2906593)](https://ieeexplore.ieee.org/document/8675433) ### Copyright and Terms of Use From 730233fec0d9ca2d2df7111398f8fff89acb2e3b Mon Sep 17 00:00:00 2001 From: Oliver Sanders Date: Tue, 2 Aug 2022 11:12:59 +0100 Subject: [PATCH 04/66] Update PULL_REQUEST_TEMPLATE.md Simplify PR template (post Cylc 7) and add an item for --- .github/PULL_REQUEST_TEMPLATE.md | 32 +++++++++++--------------------- 1 file changed, 11 insertions(+), 21 deletions(-) diff --git a/.github/PULL_REQUEST_TEMPLATE.md b/.github/PULL_REQUEST_TEMPLATE.md index ddcd2a64883..c013413882f 100644 --- a/.github/PULL_REQUEST_TEMPLATE.md +++ b/.github/PULL_REQUEST_TEMPLATE.md @@ -1,27 +1,17 @@ - + - +**Check List** -These changes partially address #xxxx -These changes close #xxxx -This is a small change with no associated Issue. - - - - -**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). -- [ ] Already covered by existing tests. -- [ ] Does not need tests (why?). - -- [ ] Appropriate change log entry included. -- [ ] No change log entry required (why? e.g. invisible to users). - -- [ ] (master branch) I have opened a documentation PR at cylc/cylc-doc/pull/XXXX. -- [ ] (7.8.x branch) I have updated the documentation in this PR branch. -- [ ] No documentation update required. +- [ ] Tests are included if required. +- [ ] Changelog is included if required. +- [ ] [Cylc-Doc](https://github.com/cylc/cylc-doc) pull request opened if required at cylc/cylc-doc/pull/XXXX. +- [ ] [if bugfix] PRs raised to both master and the relevant bugfix branch. From 803065902ed0deb194089a4579bb8766f53d2f47 Mon Sep 17 00:00:00 2001 From: Oliver Sanders Date: Tue, 2 Aug 2022 11:17:22 +0100 Subject: [PATCH 05/66] Update README.md --- README.md | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index ce4207cd47c..17cd2720345 100644 --- a/README.md +++ b/README.md @@ -37,7 +37,10 @@ cat > ~/cylc-src/example/flow.cylc <<__CONFIG__ initial cycle point = 1 cycling mode = integer [[graph]] - P1 = a => b[-P1] => c & d + P1 = """ + a => b => c & d + b[-P1] => b + """ [runtime] [[a, b, c, d]] script = echo "Hello $CYLC_TASK_NAME" From 9aa9e1aee168a8b30d31f35794df73700dc6487b Mon Sep 17 00:00:00 2001 From: Oliver Sanders Date: Wed, 3 Aug 2022 09:06:53 +0100 Subject: [PATCH 06/66] Apply suggestions from code review Co-authored-by: Hilary James Oliver --- .github/PULL_REQUEST_TEMPLATE.md | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/.github/PULL_REQUEST_TEMPLATE.md b/.github/PULL_REQUEST_TEMPLATE.md index c013413882f..ca7954e4691 100644 --- a/.github/PULL_REQUEST_TEMPLATE.md +++ b/.github/PULL_REQUEST_TEMPLATE.md @@ -1,6 +1,7 @@ @@ -11,7 +12,7 @@ Thanks for your contribution: - [ ] 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`. -- [ ] Tests are included if required. -- [ ] Changelog is included if required. +- [ ] Tests are included (or explain why tests are not needed). +- [ ] `CHANGES.md` entry included if this is a significant change - [ ] [Cylc-Doc](https://github.com/cylc/cylc-doc) pull request opened if required at cylc/cylc-doc/pull/XXXX. -- [ ] [if bugfix] PRs raised to both master and the relevant bugfix branch. +- [ ] If this is a bug fix, PRs raised to both master and the relevant maintenance branch. From 3ace0f729b8de2ba1f2e21f9b72c740fce446d4e Mon Sep 17 00:00:00 2001 From: Oliver Sanders Date: Wed, 3 Aug 2022 09:08:43 +0100 Subject: [PATCH 07/66] Apply suggestions from code review --- README.md | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index 17cd2720345..14d965364b4 100644 --- a/README.md +++ b/README.md @@ -17,8 +17,9 @@ Cylc (pronounced silk) is a general purpose workflow engine that specialises in cycling workflows and has strong scaling characteristics. -Cylc was originally developed to meet the challenges of production weather -forecasting - which is notorious for the size and complexity of its workflows. +Cylc is a general purpose workflow engine that also orchestrates cycling systems very efficiently. +It is used in production weather, climate, and environmental forecasting on HPC, but is +not specialized to those domains. ### Quick Start From 6ed04e0832147aa36c28dde74b5c777ec4f0bad3 Mon Sep 17 00:00:00 2001 From: Oliver Sanders Date: Wed, 3 Aug 2022 13:20:39 +0100 Subject: [PATCH 08/66] reload: preserve xtriggers on reload * Closes https://github.com/cylc/cylc-flow/issues/4866 * Reload causes new instances of each task to be created. * The attributes of the old instances are copied onto the new ones. * Xtriggers were not copied so were lost. * This means that all xtriggers were effectively deleted on reload. --- cylc/flow/task_proxy.py | 7 +++ tests/functional/reload/25-xtriggers.t | 72 ++++++++++++++++++++++++++ 2 files changed, 79 insertions(+) create mode 100644 tests/functional/reload/25-xtriggers.t diff --git a/cylc/flow/task_proxy.py b/cylc/flow/task_proxy.py index 966b17ad74b..b5511ffb188 100644 --- a/cylc/flow/task_proxy.py +++ b/cylc/flow/task_proxy.py @@ -284,6 +284,13 @@ def copy_to_reload_successor(self, reload_successor): reload_successor.state.is_runahead = self.state.is_runahead reload_successor.state.is_updated = self.state.is_updated reload_successor.state.prerequisites = self.state.prerequisites + reload_successor.state.xtriggers.update({ + # copy across any special "_cylc" xtriggers which were added + # dynamically at runtime (i.e. execution retry xtriggers) + key: value + for key, value in self.state.xtriggers.items() + if key.startswith('_cylc') + }) reload_successor.jobs = self.jobs @staticmethod diff --git a/tests/functional/reload/25-xtriggers.t b/tests/functional/reload/25-xtriggers.t new file mode 100644 index 00000000000..0eecc13590d --- /dev/null +++ b/tests/functional/reload/25-xtriggers.t @@ -0,0 +1,72 @@ +#!/usr/bin/env bash +# THIS FILE IS PART OF THE CYLC WORKFLOW ENGINE. +# Copyright (C) NIWA & British Crown (Met Office) & Contributors. +# +# This program is free software: you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation, either version 3 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program. If not, see . +#------------------------------------------------------------------------------- + +# Ensure that xtriggers are preserved after reloads +# See https://github.com/cylc/cylc-flow/issues/4866 + +. "$(dirname "$0")/test_header" + +set_test_number 6 + +init_workflow "${TEST_NAME_BASE}" <<'__FLOW_CONFIG__' +[scheduling] + [[graph]] + R1 = """ + broken + reload + """ + +[runtime] + [[broken]] + script = false + # should be long enough for the reload to complete + # (annoyingly we can't make this event driven) + execution retry delays = PT1M + # NOTE: "execution retry delays" is implemented as an xtrigger + + [[reload]] + script = """ + # wait for "broken" to fail + cylc__job__poll_grep_workflow_log \ + '1/broken .* (received)failed/ERR' + # fix "broken" to allow it to pass + sed -i 's/false/true/' "${CYLC_WORKFLOW_RUN_DIR}/flow.cylc" + # reload the workflow + cylc reload "${CYLC_WORKFLOW_ID}" + """ +__FLOW_CONFIG__ + +run_ok "${TEST_NAME_BASE}-val" cylc validate "${WORKFLOW_NAME}" +workflow_run_ok "${TEST_NAME_BASE}-run" cylc play "${WORKFLOW_NAME}" --no-detach + +# ensure the following order of events +# 1. "1/broken" fails +# 2. workflow is reloaded (by "1/reload") +# 3. the retry xtrigger for "1/broken" becomes satisfied (after the reload) +# (thus proving that the xtrigger survived the reload) +# 4. "1/broken" succeeds +log_scan "${TEST_NAME_BASE}-scan" \ + "$(cylc cat-log -m p "${WORKFLOW_NAME}")" \ + 1 1 \ + '1/broken .* (received)failed/ERR' \ + 'Command succeeded: reload_workflow()' \ + 'xtrigger satisfied: _cylc_retry_1/broken' \ + '1/broken .* (received)succeeded' + +purge +exit From 41ead8f72cd469e483ae63571f71e1de256d1343 Mon Sep 17 00:00:00 2001 From: Hilary James Oliver Date: Thu, 4 Aug 2022 09:54:35 +1200 Subject: [PATCH 09/66] Update .github/PULL_REQUEST_TEMPLATE.md [skip ci] Co-authored-by: Ronnie Dutta <61982285+MetRonnie@users.noreply.github.com> --- .github/PULL_REQUEST_TEMPLATE.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/PULL_REQUEST_TEMPLATE.md b/.github/PULL_REQUEST_TEMPLATE.md index ca7954e4691..1293682520f 100644 --- a/.github/PULL_REQUEST_TEMPLATE.md +++ b/.github/PULL_REQUEST_TEMPLATE.md @@ -13,6 +13,6 @@ Thanks for your contribution: - [ ] Does not contain off-topic changes (use other PRs for other changes). - [ ] Applied any dependency changes to both `setup.cfg` and `conda-environment.yml`. - [ ] Tests are included (or explain why tests are not needed). -- [ ] `CHANGES.md` entry included if this is a significant change +- [ ] `CHANGES.md` entry included if this is a change that can affect users - [ ] [Cylc-Doc](https://github.com/cylc/cylc-doc) pull request opened if required at cylc/cylc-doc/pull/XXXX. - [ ] If this is a bug fix, PRs raised to both master and the relevant maintenance branch. From b7968dd071384d9f4dce7b9890d98ff5926c42cf Mon Sep 17 00:00:00 2001 From: Hilary James Oliver Date: Thu, 4 Aug 2022 10:10:15 +1200 Subject: [PATCH 10/66] Tweak README description. --- README.md | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/README.md b/README.md index 14d965364b4..85717eda748 100644 --- a/README.md +++ b/README.md @@ -14,12 +14,10 @@ -Cylc (pronounced silk) is a general purpose workflow engine -that specialises in cycling workflows and has strong scaling characteristics. - -Cylc is a general purpose workflow engine that also orchestrates cycling systems very efficiently. -It is used in production weather, climate, and environmental forecasting on HPC, but is -not specialized to those domains. +Cylc (pronounced silk) is a general purpose workflow engine that also +manages cycling systems very efficiently. It is used in production weather, +climate, and environmental forecasting on HPC, but is not specialized to those +domains. ### Quick Start From 82bc2c9c9d38fd339f75a84e02b087e907cbc07b Mon Sep 17 00:00:00 2001 From: Ronnie Dutta <61982285+MetRonnie@users.noreply.github.com> Date: Mon, 8 Aug 2022 10:45:13 +0100 Subject: [PATCH 11/66] Improve task pool integration tests + small fixes (#5026) Improve task_pool integration tests * Closes #5012 * Unit test `WorkflowConfig.find_taskdefs()` * Fix spurious warnings for `TaskPool.match_taskdef()` * Add basic test for Resolvers.stop() * Add doctest for TaskDef `__repr__()` --- cylc/flow/config.py | 44 +++++++++--------- cylc/flow/task_pool.py | 33 +++++++++---- cylc/flow/taskdef.py | 27 +++++++---- tests/integration/conftest.py | 3 +- tests/integration/test_resolvers.py | 27 +++++++++-- tests/integration/test_task_pool.py | 53 ++++++++++----------- tests/unit/conftest.py | 2 +- tests/unit/test_config.py | 72 +++++++++++++++++++++++++++-- 8 files changed, 185 insertions(+), 76 deletions(-) diff --git a/cylc/flow/config.py b/cylc/flow/config.py index ffdcc3ff8df..685dcfe7f9f 100644 --- a/cylc/flow/config.py +++ b/cylc/flow/config.py @@ -2096,36 +2096,38 @@ def set_required_outputs( continue taskdef.set_required_output(output, not optional) - def find_taskdefs(self, name: str) -> List[TaskDef]: + def find_taskdefs(self, name: str) -> Set[TaskDef]: """Find TaskDef objects in family "name" or matching "name". Return a list of TaskDef objects which: * have names that glob matches "name". * are in a family that glob matches "name". """ - ret: List[TaskDef] = [] if name in self.taskdefs: # Match a task name - ret.append(self.taskdefs[name]) - else: - fams = self.runtime['first-parent descendants'] + return {self.taskdefs[name]} + + fams = self.runtime['first-parent descendants'] + if name in fams: # Match a family name - if name in fams: - for member in fams[name]: - if member in self.taskdefs: - ret.append(self.taskdefs[member]) - else: - # Glob match task names - for key, taskdef in self.taskdefs.items(): - if fnmatchcase(key, name): - ret.append(taskdef) - # Glob match family names - for key, members in fams.items(): - if fnmatchcase(key, name): - for member in members: - if member in self.taskdefs: - ret.append(self.taskdefs[member]) - return ret + return { + self.taskdefs[member] for member in fams[name] + if member in self.taskdefs + } + + # Glob match + from_task_names = { + taskdef for key, taskdef in self.taskdefs.items() + if fnmatchcase(key, name) + } + from_family_names = { + self.taskdefs[member] + for key, members in fams.items() + if fnmatchcase(key, name) + for member in members + if member in self.taskdefs + } + return from_task_names.union(from_family_names) def get_taskdef( self, name: str, orig_expr: Optional[str] = None diff --git a/cylc/flow/task_pool.py b/cylc/flow/task_pool.py index 252ae95223a..74259618721 100644 --- a/cylc/flow/task_pool.py +++ b/cylc/flow/task_pool.py @@ -77,6 +77,7 @@ class TaskPool: """Task pool of a workflow.""" ERR_TMPL_NO_TASKID_MATCH = "No matching tasks found: {0}" + ERR_PREFIX_TASK_NOT_ON_SEQUENCE = "Invalid cycle point for task: {0}, {1}" SUICIDE_MSG = "suicide" def __init__( @@ -1429,6 +1430,11 @@ def spawn_task( # Spawn if on-sequence and within recurrence bounds. taskdef = self.config.get_taskdef(name) if not taskdef.is_valid_point(point): + LOG.warning( + self.ERR_PREFIX_TASK_NOT_ON_SEQUENCE.format( + taskdef.name, point + ) + ) return None itask = TaskProxy( @@ -1498,8 +1504,7 @@ def force_spawn_children( Args: items: Identifiers for matching task definitions, each with the - form "name[.point][:state]" or "[point/]name[:state]". - Glob-like patterns will give a warning and be skipped. + form "point/name". outputs: List of outputs to spawn on flow_num: Flow number to attribute the outputs @@ -1766,7 +1771,6 @@ def filter_task_proxies( [self.main_pool, self.hidden_pool], ids, warn=warn, - ) future_matched: 'Set[Tuple[str, PointBase]]' = set() if future and unmatched: @@ -1844,7 +1848,11 @@ def match_future_tasks( if taskdef.is_valid_point(point): matched_tasks.add((taskdef.name, point)) else: - # is_valid_point() already logged warning + LOG.warning( + self.ERR_PREFIX_TASK_NOT_ON_SEQUENCE.format( + taskdef.name, point + ) + ) unmatched_tasks.append(id_) continue return matched_tasks, unmatched_tasks @@ -1857,8 +1865,10 @@ def match_taskdefs( Args: items: Identifiers for matching task definitions, each with the - form "name[.point][:state]" or "[point/]name[:state]". - Glob-like patterns will give a warning and be skipped. + form "point/name". + Cycle point globs will give a warning and be skipped, + but task name globs will be matched. + Task states are ignored. """ n_warnings = 0 @@ -1884,7 +1894,7 @@ def match_taskdefs( ) n_warnings += 1 continue - taskdefs: List['TaskDef'] = self.config.find_taskdefs(name_str) + taskdefs = self.config.find_taskdefs(name_str) if not taskdefs: LOG.warning( self.ERR_TMPL_NO_TASKID_MATCH.format( @@ -1898,8 +1908,13 @@ def match_taskdefs( if taskdef.is_valid_point(point): task_items[(taskdef.name, point)] = taskdef else: - # is_valid_point() already logged warning - n_warnings += 1 + if not contains_fnmatch(name_str): + LOG.warning( + self.ERR_PREFIX_TASK_NOT_ON_SEQUENCE.format( + taskdef.name, point + ) + ) + n_warnings += 1 continue return n_warnings, task_items diff --git a/cylc/flow/taskdef.py b/cylc/flow/taskdef.py index 62704a56a84..29e0f7d0de0 100644 --- a/cylc/flow/taskdef.py +++ b/cylc/flow/taskdef.py @@ -17,6 +17,7 @@ """Task definition.""" from collections import deque +from typing import TYPE_CHECKING import cylc.flow.flags from cylc.flow.exceptions import TaskDefError @@ -27,9 +28,11 @@ TASK_OUTPUT_SUCCEEDED, TASK_OUTPUT_FAILED ) -from cylc.flow import LOG from cylc.flow.task_outputs import SORT_ORDERS +if TYPE_CHECKING: + from cylc.flow.cycling import PointBase + def generate_graph_children(tdef, point): """Determine graph children of this task (for spawning).""" @@ -119,7 +122,6 @@ class TaskDef: # Store the elapsed times for a maximum of 10 cycles MAX_LEN_ELAPSED_TIMES = 10 - ERR_PREFIX_TASK_NOT_ON_SEQUENCE = "Invalid cycle point for task: " def __init__(self, name, rtcfg, run_mode, start_point, initial_point): if not TaskID.is_valid_name(name): @@ -299,16 +301,11 @@ def has_only_abs_triggers(self, point): return False return has_abs - def is_valid_point(self, point): + def is_valid_point(self, point: 'PointBase') -> bool: """Return True if point is on-sequence and within bounds.""" - is_valid_point = any( - sequence.is_valid(point) - for sequence in self.sequences + return any( + sequence.is_valid(point) for sequence in self.sequences ) - if not is_valid_point: - LOG.warning("%s%s, %s" % ( - self.ERR_PREFIX_TASK_NOT_ON_SEQUENCE, self.name, point)) - return is_valid_point def first_point(self, icp): """Return the first point for this task.""" @@ -361,3 +358,13 @@ def is_parentless(self, point): or all(x < self.start_point for x in parent_points) or self.has_only_abs_triggers(point) ) + + def __repr__(self) -> str: + """ + >>> TaskDef( + ... name='oliver', rtcfg={}, run_mode='fake', start_point='1', + ... initial_point='1' + ... ) + + """ + return f"" diff --git a/tests/integration/conftest.py b/tests/integration/conftest.py index 2fbc8c14b99..23179a73f42 100644 --- a/tests/integration/conftest.py +++ b/tests/integration/conftest.py @@ -20,7 +20,7 @@ from pathlib import Path import pytest from shutil import rmtree -from typing import List, TYPE_CHECKING, Tuple, Set +from typing import List, TYPE_CHECKING, Set, Tuple from cylc.flow.config import WorkflowConfig from cylc.flow.network.client import WorkflowRuntimeClient @@ -215,6 +215,7 @@ def mod_one_conf(): @pytest.fixture def one(one_conf, flow, scheduler): + """Return a Scheduler for the simple "R1 = one" graph.""" reg = flow(one_conf) schd = scheduler(reg) return schd diff --git a/tests/integration/test_resolvers.py b/tests/integration/test_resolvers.py index 00fa855acc8..6301861b058 100644 --- a/tests/integration/test_resolvers.py +++ b/tests/integration/test_resolvers.py @@ -14,14 +14,17 @@ # You should have received a copy of the GNU General Public License # along with this program. If not, see . -from typing import Callable -import pytest +import logging +from typing import AsyncGenerator, Callable from unittest.mock import Mock +import pytest + from cylc.flow.data_store_mgr import EDGES, TASK_PROXIES from cylc.flow.id import Tokens from cylc.flow.network.resolvers import Resolvers from cylc.flow.scheduler import Scheduler +from cylc.flow.workflow_status import StopMode @pytest.fixture @@ -51,7 +54,7 @@ async def mock_flow( mod_flow: Callable[..., str], mod_scheduler: Callable[..., Scheduler], mod_start, -) -> Scheduler: +) -> AsyncGenerator[Scheduler, None]: ret = Mock() ret.reg = mod_flow({ 'scheduler': { @@ -216,3 +219,21 @@ async def test_mutation_mapper(mock_flow): assert response is None with pytest.raises(ValueError): await mock_flow.resolvers._mutation_mapper('non_exist', {}, meta) + + +@pytest.mark.asyncio +async def test_stop( + one: Scheduler, run: Callable, log_filter: Callable, +): + """Test the stop resolver.""" + async with run(one) as log: + resolvers = Resolvers( + one.data_store_mgr, + schd=one + ) + resolvers.stop(StopMode.REQUEST_CLEAN) + one.process_command_queue() + assert log_filter( + log, level=logging.INFO, contains="Command succeeded: stop" + ) + assert one.stop_mode == StopMode.REQUEST_CLEAN diff --git a/tests/integration/test_task_pool.py b/tests/integration/test_task_pool.py index 86adf380681..1334cfcaa12 100644 --- a/tests/integration/test_task_pool.py +++ b/tests/integration/test_task_pool.py @@ -19,7 +19,7 @@ import logging import pytest from pytest import param -from typing import Callable, Iterable, List, Tuple, Union +from typing import AsyncGenerator, Callable, Iterable, List, Tuple, Union from cylc.flow.cycling import PointBase from cylc.flow.cycling.integer import IntegerPoint @@ -35,9 +35,9 @@ # NOTE: foo and bar have no parents so at start-up (even with the workflow -# paused) they get spawned out to the runahead limit. 2/pub gets spawned +# paused) they get spawned out to the runahead limit. 2/pub spawns # immediately too, because we spawn autospawn absolute-triggered tasks as -# well as parentless tasks. +# well as parentless tasks. 3/asd does not spawn at start, however. EXAMPLE_FLOW_CFG = { 'scheduler': { 'allow implicit tasks': True @@ -49,7 +49,8 @@ 'runahead limit': 'P3', 'graph': { 'P1': 'foo & bar', - 'R1/2': 'foo[1] => pub' + 'R1/2': 'foo[1] => pub', + 'R1/3': 'foo[-P1] => asd' } }, 'runtime': { @@ -62,7 +63,7 @@ def get_task_ids( name_point_list: Iterable[Tuple[str, Union[PointBase, str, int]]] ) -> List[str]: - """Helper function to return sorted task identities ("{name}.{point}") + """Helper function to return sorted task identities from a list of (name, point) tuples.""" return sorted(f'{point}/{name}' for name, point in name_point_list) @@ -111,7 +112,7 @@ async def example_flow( scheduler: Callable, start, caplog: pytest.LogCaptureFixture, -) -> Scheduler: +) -> AsyncGenerator[Scheduler, None]: """Return a scheduler for interrogating its task pool. This is function-scoped so slower than mod_example_flow; only use this @@ -131,7 +132,7 @@ async def example_flow( [ param( ['*/foo'], ['1/foo', '2/foo', '3/foo', '4/foo', '5/foo'], [], [], - id="Basic" + id="Point glob" ), param( ['1/*'], @@ -142,10 +143,6 @@ async def example_flow( ['1/FAM'], ['1/bar'], [], [], id="Family name" ), - param( - ['*/foo'], ['1/foo', '2/foo', '3/foo', '4/foo', '5/foo'], [], [], - id="Point glob" - ), param( ['*:waiting'], ['1/foo', '1/bar', '2/foo', '2/bar', '2/pub', '3/foo', '3/bar', @@ -153,7 +150,8 @@ async def example_flow( id="Task state" ), param( - ['8/foo'], [], ['8/foo'], ["No active tasks matching: 8/foo"], + ['8/foo', '3/asd'], [], ['8/foo', '3/asd'], + [f"No active tasks matching: {x}" for x in ['8/foo', '3/asd']], id="Task not yet spawned" ), param( @@ -163,15 +161,14 @@ async def example_flow( ), param( ['1/grogu', '*/grogu'], [], ['1/grogu', '*/grogu'], - ["No active tasks matching: 1/grogu", - "No active tasks matching: */grogu"], + [f"No active tasks matching: {x}" for x in ['1/grogu', '*/grogu']], id="No such task" ), param( ['*'], ['1/foo', '1/bar', '2/foo', '2/bar', '2/pub', '3/foo', '3/bar', '4/foo', '4/bar', '5/foo', '5/bar'], [], [], - id="No items given - get all tasks" + id="Glob everything" ) ] ) @@ -191,7 +188,7 @@ async def test_filter_task_proxies( Params: items: Arg passed to filter_task_proxies(). expected_task_ids: IDs of the TaskProxys that are expected to be - returned, of the form "{point}/{name}"/ + returned. expected_bad_items: Expected to be returned. expected_warnings: Expected to be logged. """ @@ -266,9 +263,13 @@ async def test_filter_task_proxies_hidden( id="Basic" ), param( - ['2/*'], ['2/foo', '2/bar', '2/pub'], [], + ['3/*', '1/f*'], ['3/foo', '3/bar', '3/asd', '1/foo'], [], id="Name glob" ), + param( + ['3'], ['3/foo', '3/bar', '3/asd'], [], + id="No name" + ), param( ['2/FAM'], ['2/bar'], [], id="Family name" @@ -278,7 +279,8 @@ async def test_filter_task_proxies_hidden( id="Point glob not allowed" ), param( - ['1/grogu'], [], ["No matching tasks found: 1/grogu"], + ['1/grogu', '1/gro*'], [], + [f"No matching tasks found: {x}" for x in ['1/grogu', '1/gro*']], id="No such task" ), param( @@ -331,12 +333,12 @@ async def test_match_taskdefs( 'items, expected_tasks_to_hold_ids, expected_warnings', [ param( - ['1/foo', '2/foo'], ['1/foo', '2/foo'], [], + ['1/foo', '3/asd'], ['1/foo', '3/asd'], [], id="Active & future tasks" ), param( - ['1/*', '2/*', '6/*'], - ['1/foo', '1/bar', '2/foo', '2/bar', '2/pub'], + ['1/*', '2/*', '3/*', '6/*'], + ['1/foo', '1/bar', '2/foo', '2/bar', '2/pub', '3/foo', '3/bar'], ["No active tasks matching: 6/*"], id="Name globs hold active tasks only" # (active means n=0 here) ), @@ -403,7 +405,7 @@ async def test_release_held_tasks( """Test TaskPool.release_held_tasks(). For a workflow with held active tasks 1/foo & 1/bar, and held future task - 2/pub. + 3/asd. We skip testing the matching logic here because it would be slow using the function-scoped example_flow fixture, and it would repeat what is covered @@ -411,7 +413,7 @@ async def test_release_held_tasks( """ # Setup task_pool = example_flow.pool - expected_tasks_to_hold_ids = sorted(['1/foo', '1/bar', '2/pub']) + expected_tasks_to_hold_ids = sorted(['1/foo', '1/bar', '3/asd']) task_pool.hold_tasks(expected_tasks_to_hold_ids) for itask in task_pool.get_all_tasks(): hold_expected = itask.identity in expected_tasks_to_hold_ids @@ -421,10 +423,9 @@ async def test_release_held_tasks( assert get_task_ids(db_tasks_to_hold) == expected_tasks_to_hold_ids # Test - task_pool.release_held_tasks(['1/foo', '2/pub']) + task_pool.release_held_tasks(['1/foo', '3/asd']) for itask in task_pool.get_all_tasks(): - hold_expected = itask.identity == '1/bar' - assert itask.state.is_held is hold_expected + assert itask.state.is_held is (itask.identity == '1/bar') expected_tasks_to_hold_ids = sorted(['1/bar']) assert get_task_ids(task_pool.tasks_to_hold) == expected_tasks_to_hold_ids diff --git a/tests/unit/conftest.py b/tests/unit/conftest.py index 8e4390f5f7b..c066d10c064 100644 --- a/tests/unit/conftest.py +++ b/tests/unit/conftest.py @@ -123,7 +123,7 @@ def mod_tmp_run_dir(tmp_path_factory: pytest.TempPathFactory): """Module-scoped version of tmp_run_dir()""" tmp_path = tmp_path_factory.getbasetemp() with pytest.MonkeyPatch.context() as mp: - return _tmp_run_dir(tmp_path, mp) + yield _tmp_run_dir(tmp_path, mp) def _tmp_src_dir(tmp_path: Path): diff --git a/tests/unit/test_config.py b/tests/unit/test_config.py index 919c952b2cc..05f047e0775 100644 --- a/tests/unit/test_config.py +++ b/tests/unit/test_config.py @@ -15,7 +15,7 @@ # along with this program. If not, see . from optparse import Values -from typing import Any, Callable, Dict, Optional, Tuple, Type +from typing import Any, Callable, Dict, List, Optional, Tuple, Type from pathlib import Path import pytest import logging @@ -46,8 +46,7 @@ Fixture = Any -@pytest.fixture -def tmp_flow_config(tmp_run_dir: Callable): +def _tmp_flow_config(tmp_run_dir: Callable): """Create a temporary flow config file for use in init'ing WorkflowConfig. Args: @@ -56,12 +55,22 @@ def tmp_flow_config(tmp_run_dir: Callable): Returns the path to the flow file. """ - def _tmp_flow_config(reg: str, config: str) -> Path: + def __tmp_flow_config(reg: str, config: str) -> Path: run_dir: Path = tmp_run_dir(reg) flow_file = run_dir / WorkflowFiles.FLOW_FILE flow_file.write_text(config) return flow_file - return _tmp_flow_config + return __tmp_flow_config + + +@pytest.fixture +def tmp_flow_config(tmp_run_dir: Callable): + return _tmp_flow_config(tmp_run_dir) + + +@pytest.fixture(scope='module') +def mod_tmp_flow_config(mod_tmp_run_dir: Callable): + return _tmp_flow_config(mod_tmp_run_dir) class TestWorkflowConfig: @@ -1439,3 +1448,56 @@ def test_check_for_owner(runtime_cfg): else: # Assert function doesn't raise if no owner set: assert WorkflowConfig.check_for_owner(runtime_cfg) is None + + +@pytest.fixture(scope='module') +def awe_config(mod_tmp_flow_config: Callable) -> WorkflowConfig: + """Return a workflow config object.""" + reg = 'awe' + flow_file = mod_tmp_flow_config(reg, ''' + [scheduling] + cycling mode = integer + [[graph]] + P1 = ordinary & sterling + R1/2 = fra_mauro + [runtime] + [[USA, MOON]] + [[ordinary, sterling]] + inherit = USA + [[fra_mauro]] + inherit = MOON + ''') + return WorkflowConfig( + workflow=reg, fpath=flow_file, options=ValidateOptions() + ) + + +@pytest.mark.parametrize( + 'name, expected', + [ + pytest.param( + 'ordinary', ['ordinary'], id="task name" + ), + pytest.param( + 'USA', ['ordinary', 'sterling'], id="family name" + ), + pytest.param( + 'fra*', ['fra_mauro'], id="glob task name" + ), + pytest.param( + 'U*', ['ordinary', 'sterling'], id="glob family name" + ), + pytest.param( + '*', ['ordinary', 'sterling', 'fra_mauro'], id="glob everything" + ), + pytest.param( + 'butte', [], id="no match" + ), + ] +) +def test_find_taskdefs( + name: str, expected: List[str], awe_config: WorkflowConfig +): + assert sorted( + t.name for t in awe_config.find_taskdefs(name) + ) == sorted(expected) From 37ca3db872db873e4e123aec091cb7815774e14e Mon Sep 17 00:00:00 2001 From: Hilary James Oliver Date: Mon, 8 Aug 2022 21:58:12 +1200 Subject: [PATCH 12/66] Shorten error msg (#5046) * Shorten an error message. * Even better than that. --- cylc/flow/pathutil.py | 12 ++++++++---- tests/unit/test_pathutil.py | 4 ++-- 2 files changed, 10 insertions(+), 6 deletions(-) diff --git a/cylc/flow/pathutil.py b/cylc/flow/pathutil.py index 5b7fc37f18c..0d62ef37ad4 100644 --- a/cylc/flow/pathutil.py +++ b/cylc/flow/pathutil.py @@ -42,6 +42,8 @@ ) """Matches relative paths that are explicit (starts with ./)""" +SHELL_ENV_VARS = re.compile(r'\$[^$/]*') + def expand_path(*args: Union[Path, str]) -> str: """Expand both vars and user in path and normalise it, joining any @@ -174,11 +176,13 @@ def make_localhost_symlinks( else: symlink_path = os.path.join(rund, key) target = expand_path(value) - if '$' in target: + env_vars = SHELL_ENV_VARS.findall(target) + if env_vars: raise WorkflowFilesError( - f'Unable to create symlink to {target}.' - f" '{value}' contains an invalid environment variable." - ' Please check configuration.') + f"Can't symlink to {target}\n" + "Undefined variables, check " + f"global config: {', '.join(env_vars)}") + symlink_success = make_symlink_dir(symlink_path, target) # Symlink info returned for logging purposes. Symlinks should be # created before logs as the log dir may be a symlink. diff --git a/tests/unit/test_pathutil.py b/tests/unit/test_pathutil.py index 2909ff916e5..67a779e6b7e 100644 --- a/tests/unit/test_pathutil.py +++ b/tests/unit/test_pathutil.py @@ -320,8 +320,8 @@ def test_incorrect_environment_variables_raise_error( with pytest.raises(WorkflowFilesError) as excinfo: make_localhost_symlinks('rund', 'test_workflow') assert ( - "'$doh/cylc-run/test_workflow' contains an invalid " - "environment variable" + "Can't symlink to $doh/cylc-run/test_workflow\n" + "Undefined variables, check global config: $doh" ) in str(excinfo.value) From b04fb77a2a2bf522d142a1fe270f6b76dfecf345 Mon Sep 17 00:00:00 2001 From: Oliver Sanders Date: Mon, 8 Aug 2022 11:09:35 +0100 Subject: [PATCH 13/66] scan: switch from pyuv to os.listdir in asyncio executor (#5028) * We were using pyuv to perform directory listing. * Unfortunately pyuv is no longer actively maintained. * Switching to the simpler method of running `os.listdir` in an executor ala aiofiles. * Unblocks #4524 --- conda-environment.yml | 1 - cylc/flow/async_util.py | 25 ++++++++----------------- setup.cfg | 2 +- tests/integration/test_async_util.py | 2 +- 4 files changed, 10 insertions(+), 20 deletions(-) diff --git a/conda-environment.yml b/conda-environment.yml index 48e6c8cbb91..00ccbd27a53 100644 --- a/conda-environment.yml +++ b/conda-environment.yml @@ -15,7 +15,6 @@ dependencies: - protobuf >=3.19.0,<3.20.0 - psutil >=5.6.0 - python - - pyuv >=1.4.0,<1.5.0 - pyzmq >=22,<23 - setuptools >=49 - urwid >=2,<3 diff --git a/cylc/flow/async_util.py b/cylc/flow/async_util.py index c1068c867f1..aa62b39acc7 100644 --- a/cylc/flow/async_util.py +++ b/cylc/flow/async_util.py @@ -16,11 +16,11 @@ """Utilities for use with asynchronous code.""" import asyncio -from functools import partial +import os from pathlib import Path from typing import List, Union -import pyuv +from aiofiles.os import wrap # type: ignore[attr-defined] from cylc.flow import LOG @@ -393,24 +393,15 @@ def _pipe(func): return _pipe -def _scandir(future, path, request): - """Callback helper for scandir().""" - future.set_result([ - Path(path, directory.name) - # request.result is None for empty dirs - for directory in request.result or [] - ]) +async_listdir = wrap(os.listdir) async def scandir(path: Union[Path, str]) -> List[Path]: - """Asynchronous directory listing using pyuv.""" - ret: asyncio.Future[List[Path]] = asyncio.Future() - - loop = pyuv.Loop.default_loop() - pyuv.fs.scandir(loop, str(path), callback=partial(_scandir, ret, path)) - loop.run() - - return await ret + """Asynchronous directory listing (performs os.listdir in an executor).""" + return [ + Path(path, sub_path) + for sub_path in await async_listdir(path) + ] async def asyncqgen(queue): diff --git a/setup.cfg b/setup.cfg index 360071b368a..7cebd39173d 100644 --- a/setup.cfg +++ b/setup.cfg @@ -70,7 +70,6 @@ install_requires = # Constrain protobuf version for compatible Scheduler-UIS comms across hosts protobuf==3.19.* psutil>=5.6.0 - pyuv==1.4.* pyzmq==22.* setuptools>=49 urwid==2.* @@ -121,6 +120,7 @@ tests = testfixtures>=6.11.0 # Type annotation stubs # http://mypy-lang.blogspot.com/2021/05/the-upcoming-switch-to-modular-typeshed.html + types-aiofiles>=0.7.0 types-Jinja2>=0.1.3 types-aiofiles>=0.1.3 types-pkg_resources>=0.1.2 diff --git a/tests/integration/test_async_util.py b/tests/integration/test_async_util.py index ec9788ef19d..6c9bfaa565e 100644 --- a/tests/integration/test_async_util.py +++ b/tests/integration/test_async_util.py @@ -34,7 +34,7 @@ def directory(tmp_path): async def test_scandir(directory): """It should list directory contents (including symlinks).""" - assert await scandir(directory) == [ + assert sorted(await scandir(directory)) == [ Path(directory, 'a'), Path(directory, 'b'), Path(directory, 'c') From 36a572b373d3a650a52307ea1463fb291b732a67 Mon Sep 17 00:00:00 2001 From: Oliver Sanders Date: Fri, 19 Nov 2021 13:20:30 +0000 Subject: [PATCH 14/66] py: support cPython 3.10 --- .github/workflows/test_fast.yml | 2 +- .github/workflows/test_functional.yml | 7 +++++++ .github/workflows/test_manylinux.yml | 2 +- setup.cfg | 1 + 4 files changed, 10 insertions(+), 2 deletions(-) diff --git a/.github/workflows/test_fast.yml b/.github/workflows/test_fast.yml index ac9bfd9b201..97ac3bb0fc2 100644 --- a/.github/workflows/test_fast.yml +++ b/.github/workflows/test_fast.yml @@ -18,7 +18,7 @@ jobs: fail-fast: false # Don't let a failed MacOS run stop the Ubuntu runs matrix: os: ['ubuntu-latest'] - python-version: ['3.7', '3.8', '3.9'] + python-version: ['3.7', '3.8', '3.9', '3.10'] include: - os: 'macos-latest' python-version: '3.7' diff --git a/.github/workflows/test_functional.yml b/.github/workflows/test_functional.yml index fc21a7d4855..1d01a420e49 100644 --- a/.github/workflows/test_functional.yml +++ b/.github/workflows/test_functional.yml @@ -43,6 +43,13 @@ jobs: platform: ['_local_background* _local_at*'] # NOTE: includes must define ALL of the matrix values include: + # latest python + - name: 'py-3.10' + os: 'ubuntu-latest' + python-version: '3.10' + test-base: 'tests/f' + chunk: '1/4' + platform: '_local_background*' # tests/k - name: 'flaky' os: 'ubuntu-latest' diff --git a/.github/workflows/test_manylinux.yml b/.github/workflows/test_manylinux.yml index bffe37465be..3a22636f59e 100644 --- a/.github/workflows/test_manylinux.yml +++ b/.github/workflows/test_manylinux.yml @@ -34,7 +34,7 @@ jobs: matrix: manylinux: ['1'] os: ['ubuntu-18.04'] # run on the oldest linux we have access to - python-version: ['3.7', '3.8', '3.9'] + python-version: ['3.7', '3.8', '3.9', '3.10'] steps: - name: Checkout diff --git a/setup.cfg b/setup.cfg index 7cebd39173d..2678bfe5f0f 100644 --- a/setup.cfg +++ b/setup.cfg @@ -50,6 +50,7 @@ classifiers = Programming Language :: Python :: 3.7 Programming Language :: Python :: 3.8 Programming Language :: Python :: 3.9 + Programming Language :: Python :: 3.10 Programming Language :: Python :: 3 :: Only Programming Language :: Python :: Implementation :: CPython Topic :: Scientific/Engineering :: Atmospheric Science From 4ddb3eee0f416b7cf0ba1ce5054e35c5d50892d1 Mon Sep 17 00:00:00 2001 From: Tim Pillinger <26465611+wxtim@users.noreply.github.com> Date: Fri, 12 Aug 2022 10:11:41 +0100 Subject: [PATCH 15/66] small flake8 fixes (#5058) --- cylc/flow/network/__init__.py | 4 ++-- cylc/flow/parsec/fileparse.py | 2 +- cylc/flow/tui/app.py | 2 +- cylc/flow/workflow_files.py | 3 ++- cylc/flow/xtriggers/echo.py | 3 ++- 5 files changed, 8 insertions(+), 6 deletions(-) diff --git a/cylc/flow/network/__init__.py b/cylc/flow/network/__init__.py index e456de3fb12..e12b1d5ef61 100644 --- a/cylc/flow/network/__init__.py +++ b/cylc/flow/network/__init__.py @@ -179,13 +179,13 @@ def _socket_bind(self, min_port, max_port, srv_prv_key_loc=None): try: server_public_key, server_private_key = zmq.auth.load_certificate( srv_prv_key_info.full_key_path) - except (ValueError): + except ValueError: raise ServiceFileError( f"Failed to find server's public " f"key in " f"{srv_prv_key_info.full_key_path}." ) - except(OSError): + except OSError: raise ServiceFileError( f"IO error opening server's private " f"key from " diff --git a/cylc/flow/parsec/fileparse.py b/cylc/flow/parsec/fileparse.py index 326aed43a62..f3e93311878 100644 --- a/cylc/flow/parsec/fileparse.py +++ b/cylc/flow/parsec/fileparse.py @@ -292,7 +292,7 @@ def process_plugins(fpath, opts): f"{extra_vars['templating_detected']} and " f"{plugin_result['templating_detected']}" ) - elif( + elif ( 'templating_detected' in plugin_result and plugin_result['templating_detected'] is not None ): diff --git a/cylc/flow/tui/app.py b/cylc/flow/tui/app.py index 484fd3fc610..fd05a613443 100644 --- a/cylc/flow/tui/app.py +++ b/cylc/flow/tui/app.py @@ -316,7 +316,7 @@ def get_snapshot(self): # Distinguish stopped flow from non-existent flow. self.client = None full_path = Path(get_workflow_run_dir(self.reg)) - if( + if ( (full_path / WorkflowFiles.SUITE_RC).is_file() or (full_path / WorkflowFiles.FLOW_FILE).is_file() ): diff --git a/cylc/flow/workflow_files.py b/cylc/flow/workflow_files.py index 344da8b5130..d0b30f826db 100644 --- a/cylc/flow/workflow_files.py +++ b/cylc/flow/workflow_files.py @@ -284,7 +284,8 @@ class ContactFileFields: """The process ID of the running workflow on ``CYLC_WORKFLOW_HOST``.""" COMMAND = 'CYLC_WORKFLOW_COMMAND' - """The command that was used to run the workflow on ``CYLC_WORKFLOW_HOST```. + """The command that was used to run the workflow on + ``CYLC_WORKFLOW_HOST```. Note that this command may be affected by: diff --git a/cylc/flow/xtriggers/echo.py b/cylc/flow/xtriggers/echo.py index eb60bb2be4d..e3c5880cab0 100644 --- a/cylc/flow/xtriggers/echo.py +++ b/cylc/flow/xtriggers/echo.py @@ -20,7 +20,8 @@ def echo(*args, **kwargs): - """Prints args to stdout and return success only if kwargs['succeed'] is True. + """Prints args to stdout and return success only if kwargs['succeed'] + is True. This may be a useful aid to understanding how xtriggers work. From 43987c37480cd4f937874eb4ee2afa2a7fcdbf4f Mon Sep 17 00:00:00 2001 From: Oliver Sanders Date: Wed, 3 Aug 2022 14:58:33 +0100 Subject: [PATCH 16/66] coverage: exclude abstractmethods --- .coveragerc | 3 +++ 1 file changed, 3 insertions(+) diff --git a/.coveragerc b/.coveragerc index 4186fd00ee4..1e2a2fb5157 100644 --- a/.coveragerc +++ b/.coveragerc @@ -35,6 +35,9 @@ exclude_lines = # Don't complain about ellipsis (exception classes, typing overloads etc): \.\.\. + # Ignore abstract methods + @(abc\.)?abstractmethod + fail_under=0 ignore_errors = False omit = From dfaf2dcd17a6fc810b1ebd076eb76dc095258e9e Mon Sep 17 00:00:00 2001 From: Oliver Sanders Date: Wed, 3 Aug 2022 15:21:43 +0100 Subject: [PATCH 17/66] tests: unittest -> pytest * Convert some old unittests to pytests --- tests/unit/cycling/test_cycling.py | 91 ++++---- tests/unit/cycling/test_integer.py | 321 +++++++++++++++-------------- 2 files changed, 209 insertions(+), 203 deletions(-) diff --git a/tests/unit/cycling/test_cycling.py b/tests/unit/cycling/test_cycling.py index 8d265d980b1..20912399926 100644 --- a/tests/unit/cycling/test_cycling.py +++ b/tests/unit/cycling/test_cycling.py @@ -14,60 +14,61 @@ # You should have received a copy of the GNU General Public License # along with this program. If not, see . -import unittest +import pytest from cylc.flow.cycling import ( - SequenceBase, IntervalBase, PointBase, parse_exclusion) + SequenceBase, + IntervalBase, + PointBase, + parse_exclusion, +) -class TestBaseClasses(unittest.TestCase): - """Test the abstract base classes cannot be instantiated on their own - """ +def test_simple_abstract_class_test(): + """Cannot instantiate abstract classes, they must be defined in + the subclasses""" + with pytest.raises(TypeError): + SequenceBase('sequence-string', 'context_string') + with pytest.raises(TypeError): + IntervalBase('value') + with pytest.raises(TypeError): + PointBase('value') - def test_simple_abstract_class_test(self): - """Cannot instantiate abstract classes, they must be defined in - the subclasses""" - self.assertRaises(TypeError, SequenceBase, "sequence-string", - "context_string") - self.assertRaises(TypeError, IntervalBase, "value") - self.assertRaises(TypeError, PointBase, "value") +def test_parse_exclusion_simple(): + """Tests the simple case of exclusion parsing""" + expression = "PT1H!20000101T02Z" + sequence, exclusion = parse_exclusion(expression) + assert sequence == "PT1H" + assert exclusion == ['20000101T02Z'] -class TestParseExclusion(unittest.TestCase): - """Test cases for the parser function""" - def test_parse_exclusion_simple(self): - """Tests the simple case of exclusion parsing""" - expression = "PT1H!20000101T02Z" - sequence, exclusion = parse_exclusion(expression) +def test_parse_exclusions_list(): + """Tests the simple case of exclusion parsing""" + expression = "PT1H!(T03, T06, T09)" + sequence, exclusion = parse_exclusion(expression) + assert sequence == "PT1H" + assert exclusion == ['T03', 'T06', 'T09'] - self.assertEqual(sequence, "PT1H") - self.assertEqual(exclusion, ['20000101T02Z']) - def test_parse_exclusions_list(self): - """Tests the simple case of exclusion parsing""" - expression = "PT1H!(T03, T06, T09)" - sequence, exclusion = parse_exclusion(expression) +def test_parse_exclusions_list_spaces(): + """Tests the simple case of exclusion parsing""" + expression = "PT1H! (T03, T06, T09) " + sequence, exclusion = parse_exclusion(expression) + assert sequence == "PT1H" + assert exclusion == ['T03', 'T06', 'T09'] - self.assertEqual(sequence, "PT1H") - self.assertEqual(exclusion, ['T03', 'T06', 'T09']) - def test_parse_exclusions_list_spaces(self): - """Tests the simple case of exclusion parsing""" - expression = "PT1H! (T03, T06, T09) " - sequence, exclusion = parse_exclusion(expression) - - self.assertEqual(sequence, "PT1H") - self.assertEqual(exclusion, ['T03', 'T06', 'T09']) - - def test_parse_bad_exclusion(self): - """Tests incorrectly formatted exclusions""" - expression1 = "T01/PT1H!(T06, T09), PT5M" - expression2 = "T01/PT1H!T03, PT17H, (T06, T09), PT5M" - expression3 = "T01/PT1H! PT8H, (T06, T09)" - expression4 = "T01/PT1H! T03, T06, T09" - - self.assertRaises(Exception, parse_exclusion, expression1) - self.assertRaises(Exception, parse_exclusion, expression2) - self.assertRaises(Exception, parse_exclusion, expression3) - self.assertRaises(Exception, parse_exclusion, expression4) +@pytest.mark.parametrize( + 'expression', + [ + 'T01/PT1H!(T06, T09), PT5M', + 'T01/PT1H!T03, PT17H, (T06, T09), PT5M', + 'T01/PT1H! PT8H, (T06, T09)', + 'T01/PT1H! T03, T06, T09', + ], +) +def test_parse_bad_exclusion(expression): + """Tests incorrectly formatted exclusions""" + with pytest.raises(Exception): + parse_exclusion(expression) diff --git a/tests/unit/cycling/test_integer.py b/tests/unit/cycling/test_integer.py index f1ce0ce86a3..0ad1a452fa4 100644 --- a/tests/unit/cycling/test_integer.py +++ b/tests/unit/cycling/test_integer.py @@ -14,163 +14,168 @@ # You should have received a copy of the GNU General Public License # along with this program. If not, see . -import unittest +import pytest from cylc.flow.cycling.integer import ( - IntegerSequence, IntegerPoint, IntegerInterval) - - -class TestIntegerSequence(unittest.TestCase): - """Contains unit tests for the IntegerSequence class.""" - - def test_exclusions_simple(self): - """Test the generation of points for integer sequences with exclusions. - """ - sequence = IntegerSequence('R/P1!3', 1, 5) - output = [] - point = sequence.get_start_point() - while point: - output.append(point) - point = sequence.get_next_point(point) - self.assertEqual([int(out) for out in output], [1, 2, 4, 5]) - - def test_multiple_exclusions_simple(self): - """Tests the multiple exclusion syntax for integer notation""" - sequence = IntegerSequence('R/P1!(2,3,7)', 1, 10) - output = [] - point = sequence.get_start_point() - while point: - output.append(point) - point = sequence.get_next_point(point) - self.assertEqual([int(out) for out in output], [1, 4, 5, 6, 8, 9, 10]) - - def test_multiple_exclusions_integer_sequence(self): - """Tests the multiple exclusion syntax for integer notation""" - sequence = IntegerSequence('P1 ! P2', 1, 10) - output = [] - point = sequence.get_start_point() - while point: - output.append(point) - point = sequence.get_next_point(point) - self.assertEqual([int(out) for out in output], [2, 4, 6, 8, 10]) - - def test_multiple_exclusions_integer_sequence2(self): - """Tests the multiple exclusion syntax for integer notation""" - sequence = IntegerSequence('P1 ! +P1/P2', 1, 10) - output = [] - point = sequence.get_start_point() - while point: - output.append(point) - point = sequence.get_next_point(point) - self.assertEqual([int(out) for out in output], [1, 3, 5, 7, 9]) - - def test_multiple_exclusions_integer_sequence3(self): - """Tests the multiple exclusion syntax for integer notation""" - sequence = IntegerSequence('P1 ! (P2, 6, 8) ', 1, 10) - output = [] - point = sequence.get_start_point() - while point: - output.append(point) - point = sequence.get_next_point(point) - self.assertEqual([int(out) for out in output], [2, 4, 10]) - - def test_multiple_exclusions_integer_sequence_weird_valid_formatting(self): - """Tests the multiple exclusion syntax for integer notation""" - sequence = IntegerSequence('P1 !(P2, 6,8) ', 1, 10) - output = [] - point = sequence.get_start_point() - while point: - output.append(point) - point = sequence.get_next_point(point) - self.assertEqual([int(out) for out in output], [2, 4, 10]) - - def test_multiple_exclusions_integer_sequence_invalid_formatting(self): - """Tests the multiple exclusion syntax for integer notation""" - sequence = 'P1 !(6,8), P2 ' - self.assertRaises(Exception, IntegerSequence, sequence, 1, 10) - - def test_multiple_exclusions_extensive(self): - """Tests IntegerSequence methods for sequences with multi-exclusions""" - points = [IntegerPoint(i) for i in range(10)] - sequence = IntegerSequence('R/P1!(2,3,7)', 1, 10) - self.assertFalse(sequence.is_on_sequence(points[3])) - self.assertFalse(sequence.is_valid(points[3])) - self.assertEqual(sequence.get_prev_point(points[3]), points[1]) - self.assertEqual(sequence.get_prev_point(points[4]), points[1]) - self.assertEqual(sequence.get_nearest_prev_point(points[3]), points[1]) - self.assertEqual(sequence.get_nearest_prev_point(points[4]), points[1]) - self.assertEqual(sequence.get_next_point(points[3]), points[4]) - self.assertEqual(sequence.get_next_point(points[2]), points[4]) - self.assertEqual(sequence.get_next_point_on_sequence( - points[3]), - points[4]) - self.assertEqual(sequence.get_next_point_on_sequence( - points[6]), - points[8]) - - sequence = IntegerSequence('R/P1!(1,3,4)', 1, 10) - self.assertEqual(sequence.get_first_point(points[1]), points[2]) - self.assertEqual(sequence.get_first_point(points[0]), points[2]) - self.assertEqual(sequence.get_start_point(), points[2]) - - sequence = IntegerSequence('R/P1!(8,9,10)', 1, 10) - self.assertEqual(sequence.get_stop_point(), points[7]) - - def test_exclusions_extensive(self): - """Test IntegerSequence methods for sequences with exclusions.""" - point_0 = IntegerPoint(0) - point_1 = IntegerPoint(1) - point_2 = IntegerPoint(2) - point_3 = IntegerPoint(3) - point_4 = IntegerPoint(4) - - sequence = IntegerSequence('R/P1!3', 1, 5) - self.assertFalse(sequence.is_on_sequence(point_3)) - self.assertFalse(sequence.is_valid(point_3)) - self.assertEqual(sequence.get_prev_point(point_3), point_2) - self.assertEqual(sequence.get_prev_point(point_4), point_2) - self.assertEqual(sequence.get_nearest_prev_point(point_3), point_2) - self.assertEqual(sequence.get_nearest_prev_point(point_3), point_2) - self.assertEqual(sequence.get_next_point(point_3), point_4) - self.assertEqual(sequence.get_next_point(point_2), point_4) - self.assertEqual(sequence.get_next_point_on_sequence(point_3), point_4) - self.assertEqual(sequence.get_next_point_on_sequence(point_2), point_4) - - sequence = IntegerSequence('R/P1!1', 1, 5) - self.assertEqual(sequence.get_first_point(point_1), point_2) - self.assertEqual(sequence.get_first_point(point_0), point_2) - self.assertEqual(sequence.get_start_point(), point_2) - - sequence = IntegerSequence('R/P1!5', 1, 5) - self.assertEqual(sequence.get_stop_point(), point_4) - - def test_simple(self): - """Run some simple tests for integer cycling.""" - sequence = IntegerSequence('R/1/P3', 1, 10) - start = sequence.p_start - stop = sequence.p_stop - - # Test point generation forwards. - point = start - output = [] - while point and stop and point <= stop: - output.append(point) - point = sequence.get_next_point(point) - self.assertEqual([int(out) for out in output], [1, 4, 7, 10]) - - # Test point generation backwards. - point = stop - output = [] - while point and start and point >= start: - output.append(point) - point = sequence.get_prev_point(point) - self.assertEqual([int(out) for out in output], [10, 7, 4, 1]) - - # Test sequence comparison - sequence1 = IntegerSequence('R/1/P2', 1, 10) - sequence2 = IntegerSequence('R/1/P2', 1, 10) - self.assertEqual(sequence1, sequence2) - sequence2.set_offset(IntegerInterval('-P2')) - self.assertEqual(sequence1, sequence2) - sequence2.set_offset(IntegerInterval('-P1')) - self.assertNotEqual(sequence1, sequence2) + IntegerSequence, + IntegerPoint, + IntegerInterval, +) + + +def test_exclusions_simple(): + """Test the generation of points for integer sequences with exclusions. + """ + sequence = IntegerSequence('R/P1!3', 1, 5) + output = [] + point = sequence.get_start_point() + while point: + output.append(point) + point = sequence.get_next_point(point) + assert [int(out) for out in output] == [1, 2, 4, 5] + + +def test_multiple_exclusions_simple(): + """Tests the multiple exclusion syntax for integer notation""" + sequence = IntegerSequence('R/P1!(2,3,7)', 1, 10) + output = [] + point = sequence.get_start_point() + while point: + output.append(point) + point = sequence.get_next_point(point) + assert [int(out) for out in output] == [1, 4, 5, 6, 8, 9, 10] + + +def test_multiple_exclusions_integer_sequence(): + """Tests the multiple exclusion syntax for integer notation""" + sequence = IntegerSequence('P1 ! P2', 1, 10) + output = [] + point = sequence.get_start_point() + while point: + output.append(point) + point = sequence.get_next_point(point) + assert [int(out) for out in output] == [2, 4, 6, 8, 10] + + +def test_multiple_exclusions_integer_sequence2(): + """Tests the multiple exclusion syntax for integer notation""" + sequence = IntegerSequence('P1 ! +P1/P2', 1, 10) + output = [] + point = sequence.get_start_point() + while point: + output.append(point) + point = sequence.get_next_point(point) + assert [int(out) for out in output] == [1, 3, 5, 7, 9] + + +def test_multiple_exclusions_integer_sequence3(): + """Tests the multiple exclusion syntax for integer notation""" + sequence = IntegerSequence('P1 ! (P2, 6, 8) ', 1, 10) + output = [] + point = sequence.get_start_point() + while point: + output.append(point) + point = sequence.get_next_point(point) + assert [int(out) for out in output] == [2, 4, 10] + + +def test_multiple_exclusions_integer_sequence_weird_valid_formatting(): + """Tests the multiple exclusion syntax for integer notation""" + sequence = IntegerSequence('P1 !(P2, 6,8) ', 1, 10) + output = [] + point = sequence.get_start_point() + while point: + output.append(point) + point = sequence.get_next_point(point) + assert [int(out) for out in output] == [2, 4, 10] + + +def test_multiple_exclusions_integer_sequence_invalid_formatting(): + """Tests the multiple exclusion syntax for integer notation""" + with pytest.raises(Exception): + IntegerSequence('P1 !(6,8), P2 ', 1, 10) + + +def test_multiple_exclusions_extensive(): + """Tests IntegerSequence methods for sequences with multi-exclusions""" + points = [IntegerPoint(i) for i in range(10)] + sequence = IntegerSequence('R/P1!(2,3,7)', 1, 10) + assert not sequence.is_on_sequence(points[3]) + assert not sequence.is_valid(points[3]) + assert sequence.get_prev_point(points[3]) == points[1] + assert sequence.get_prev_point(points[4]) == points[1] + assert sequence.get_nearest_prev_point(points[3]) == points[1] + assert sequence.get_nearest_prev_point(points[4]) == points[1] + assert sequence.get_next_point(points[3]) == points[4] + assert sequence.get_next_point(points[2]) == points[4] + assert sequence.get_next_point_on_sequence(points[3]) == points[4] + assert sequence.get_next_point_on_sequence(points[6]) == points[8] + + sequence = IntegerSequence('R/P1!(1,3,4)', 1, 10) + assert sequence.get_first_point(points[1]) == points[2] + assert sequence.get_first_point(points[0]) == points[2] + assert sequence.get_start_point() == points[2] + + sequence = IntegerSequence('R/P1!(8,9,10)', 1, 10) + assert sequence.get_stop_point() == points[7] + + +def test_exclusions_extensive(): + """Test IntegerSequence methods for sequences with exclusions.""" + point_0 = IntegerPoint(0) + point_1 = IntegerPoint(1) + point_2 = IntegerPoint(2) + point_3 = IntegerPoint(3) + point_4 = IntegerPoint(4) + + sequence = IntegerSequence('R/P1!3', 1, 5) + assert not sequence.is_on_sequence(point_3) + assert not sequence.is_valid(point_3) + assert sequence.get_prev_point(point_3) == point_2 + assert sequence.get_prev_point(point_4) == point_2 + assert sequence.get_nearest_prev_point(point_3) == point_2 + assert sequence.get_nearest_prev_point(point_3) == point_2 + assert sequence.get_next_point(point_3) == point_4 + assert sequence.get_next_point(point_2) == point_4 + assert sequence.get_next_point_on_sequence(point_3) == point_4 + assert sequence.get_next_point_on_sequence(point_2) == point_4 + + sequence = IntegerSequence('R/P1!1', 1, 5) + assert sequence.get_first_point(point_1) == point_2 + assert sequence.get_first_point(point_0) == point_2 + assert sequence.get_start_point() == point_2 + + sequence = IntegerSequence('R/P1!5', 1, 5) + assert sequence.get_stop_point() == point_4 + + +def test_simple(): + """Run some simple tests for integer cycling.""" + sequence = IntegerSequence('R/1/P3', 1, 10) + start = sequence.p_start + stop = sequence.p_stop + + # Test point generation forwards. + point = start + output = [] + while point and stop and point <= stop: + output.append(point) + point = sequence.get_next_point(point) + assert [int(out) for out in output] == [1, 4, 7, 10] + + # Test point generation backwards. + point = stop + output = [] + while point and start and point >= start: + output.append(point) + point = sequence.get_prev_point(point) + assert [int(out) for out in output] == [10, 7, 4, 1] + + # Test sequence comparison + sequence1 = IntegerSequence('R/1/P2', 1, 10) + sequence2 = IntegerSequence('R/1/P2', 1, 10) + assert sequence1 == sequence2 + sequence2.set_offset(IntegerInterval('-P2')) + assert sequence1 == sequence2 + sequence2.set_offset(IntegerInterval('-P1')) + assert sequence1 != sequence2 From c07889b6fc0f3564c354f177cfa592cc72f93e33 Mon Sep 17 00:00:00 2001 From: Oliver Sanders Date: Wed, 3 Aug 2022 15:21:54 +0100 Subject: [PATCH 18/66] tests/u: fill some holes in cyclers coverage --- cylc/flow/cycling/integer.py | 22 ++++++++------ cylc/flow/exceptions.py | 4 +++ tests/unit/cycling/test_cycling.py | 1 + tests/unit/cycling/test_integer.py | 47 ++++++++++++++++++++++++++++++ 4 files changed, 65 insertions(+), 9 deletions(-) diff --git a/cylc/flow/cycling/integer.py b/cylc/flow/cycling/integer.py index f63458372f0..749c651fc08 100644 --- a/cylc/flow/cycling/integer.py +++ b/cylc/flow/cycling/integer.py @@ -24,7 +24,10 @@ PointBase, IntervalBase, SequenceBase, ExclusionBase, parse_exclusion, cmp ) from cylc.flow.exceptions import ( - CylcMissingContextPointError, PointParsingError, IntervalParsingError + CylcMissingContextPointError, + IntervalParsingError, + PointParsingError, + SequenceParsingError, ) CYCLER_TYPE_INTEGER = "integer" @@ -201,7 +204,9 @@ def _cmp(self, other: 'IntegerInterval') -> int: def sub(self, other): """Subtract other from self as integers.""" - return IntegerInterval.from_integer(int(self) - int(other)) + if isinstance(other, IntegerInterval): + return IntegerInterval.from_integer(int(self) - int(other)) + return IntegerPoint(int(self) - int(other)) def __abs__(self): # Return an interval with absolute values for all properties. @@ -240,7 +245,8 @@ def build_exclusions(self, excl_points): integer_point = get_point_from_expression( point, None, - is_required=False).standardise() + is_required=False, + ).standardise() if integer_point not in self.exclusion_points: self.exclusion_points.append(integer_point) except PointParsingError: @@ -294,14 +300,12 @@ def __init__(self, dep_section, p_context_start, p_context_stop=None): self.exclusions = None - matched_recurrence = False expression, excl_points = parse_exclusion(dep_section) for rec, format_num in RECURRENCE_FORMAT_RECS: results = rec.match(expression) if not results: continue - matched_recurrence = True reps = results.groupdict().get("reps") if reps is not None: reps = int(reps) @@ -317,10 +321,10 @@ def __init__(self, dep_section, p_context_start, p_context_stop=None): start_required = (format_num in [1, 3]) end_required = (format_num in [1, 4]) break - - if not matched_recurrence: - raise ValueError( # TODO - raise appropriate exception - "ERROR, bad integer cycling format: %s" % expression) + else: + raise SequenceParsingError( + f'Invalid integer recurrence: {expression}' + ) self.p_start = get_point_from_expression( start, self.p_context_start, is_required=start_required) diff --git a/cylc/flow/exceptions.py b/cylc/flow/exceptions.py index d280d0e7f0d..89791cf0940 100644 --- a/cylc/flow/exceptions.py +++ b/cylc/flow/exceptions.py @@ -309,6 +309,10 @@ def __init__(self, *args): self, 'Incompatible value for {0}: {1}'.format(*args)) +class SequenceParsingError(CyclingError): + """Error on parsing an invalid sequence.""" + + class SequenceDegenerateError(CyclingError): """An error raised when adjacent points on a sequence are equal.""" diff --git a/tests/unit/cycling/test_cycling.py b/tests/unit/cycling/test_cycling.py index 20912399926..e21f6a3a501 100644 --- a/tests/unit/cycling/test_cycling.py +++ b/tests/unit/cycling/test_cycling.py @@ -66,6 +66,7 @@ def test_parse_exclusions_list_spaces(): 'T01/PT1H!T03, PT17H, (T06, T09), PT5M', 'T01/PT1H! PT8H, (T06, T09)', 'T01/PT1H! T03, T06, T09', + 'T01/PT1H !T03 !T06', ], ) def test_parse_bad_exclusion(expression): diff --git a/tests/unit/cycling/test_integer.py b/tests/unit/cycling/test_integer.py index 0ad1a452fa4..25f40e5b488 100644 --- a/tests/unit/cycling/test_integer.py +++ b/tests/unit/cycling/test_integer.py @@ -20,6 +20,8 @@ IntegerSequence, IntegerPoint, IntegerInterval, + IntervalParsingError, + SequenceParsingError, ) @@ -45,6 +47,13 @@ def test_multiple_exclusions_simple(): point = sequence.get_next_point(point) assert [int(out) for out in output] == [1, 4, 5, 6, 8, 9, 10] + # duplicate excluded points should be ignored + sequence1 = IntegerSequence('R/P1!(2,2,2)', 1, 10) + sequence2 = IntegerSequence('R/P1!(2)', 1, 10) + assert ( + sequence1.exclusions.exclusion_points + ) == sequence2.exclusions.exclusion_points + def test_multiple_exclusions_integer_sequence(): """Tests the multiple exclusion syntax for integer notation""" @@ -55,6 +64,7 @@ def test_multiple_exclusions_integer_sequence(): output.append(point) point = sequence.get_next_point(point) assert [int(out) for out in output] == [2, 4, 6, 8, 10] + assert sequence.exclusions[0] == IntegerSequence('P2', 1, 10) def test_multiple_exclusions_integer_sequence2(): @@ -179,3 +189,40 @@ def test_simple(): assert sequence1 == sequence2 sequence2.set_offset(IntegerInterval('-P1')) assert sequence1 != sequence2 + + +def test_interval_parsing_error(): + """It should reject invalid intervals.""" + with pytest.raises(IntervalParsingError): + IntegerInterval(42) + with pytest.raises(IntervalParsingError): + IntegerInterval('forty two') + + +def test_sequence_parsing_error(): + with pytest.raises(SequenceParsingError): + IntegerSequence('zz0+za', 1) + + +def test_interval_arithmetic(): + """It should do basic maths on integer intervals.""" + a = IntegerInterval('P2') + b = IntegerInterval('P3') + p = IntegerPoint('3') + + assert a + b == IntegerInterval('P5') + assert a + p == IntegerPoint('5') + assert a - b == IntegerInterval('-P1') + assert a - p == IntegerPoint('-1') + + assert abs(IntegerInterval('-P2')) == IntegerInterval('P2') + + assert a + IntegerInterval.get_null_offset() == a + + +def test_async_expr(): + """The async expression should run once and only once.""" + point = IntegerPoint('5') + sequence = IntegerSequence(IntegerSequence.get_async_expr(point), 1, 10) + assert sequence.get_next_point(IntegerPoint('1')) == point + assert sequence.get_next_point(IntegerPoint('5')) is None From 45b3db17dc844d6d9f893e1cbb43254f36a9f169 Mon Sep 17 00:00:00 2001 From: Tim Pillinger <26465611+wxtim@users.noreply.github.com> Date: Mon, 15 Aug 2022 13:29:59 +0100 Subject: [PATCH 19/66] Remove autodoc from `cylc/flow/install_plugins/__init__.py` (#5004) --- cylc/flow/install_plugins/__init__.py | 14 -------------- 1 file changed, 14 deletions(-) diff --git a/cylc/flow/install_plugins/__init__.py b/cylc/flow/install_plugins/__init__.py index 3c762247838..8903d38fb09 100644 --- a/cylc/flow/install_plugins/__init__.py +++ b/cylc/flow/install_plugins/__init__.py @@ -15,18 +15,4 @@ # along with this program. If not, see . """Plugins for running Python code before and after installation of a workflow. - -Built In Plugins ----------------- - -Cylc Flow provides the following pre-configure and post-install plugins: - -.. autosummary:: - :toctree: built-in - :template: docstring_only.rst - - cylc.flow.install_plugins.log_vc_info - -.. Note: Autosummary generates files in this directory, these are cleaned - up by `make clean`. """ From f37b019368dc049078632593c291a1d3b6cbbb15 Mon Sep 17 00:00:00 2001 From: Oliver Sanders Date: Fri, 10 Jun 2022 16:08:11 +0100 Subject: [PATCH 20/66] graph: don't re-process reference output * Refactors `cylc graph` a little, main change is that graphviz output is no longer achieved by re-processing the reference output using regexes. It's now generated from the graph directly. --- cylc/flow/scripts/graph.py | 487 ++++++++++++++++++++++--------------- 1 file changed, 292 insertions(+), 195 deletions(-) diff --git a/cylc/flow/scripts/graph.py b/cylc/flow/scripts/graph.py index 853d386535d..aa0c8dda48b 100755 --- a/cylc/flow/scripts/graph.py +++ b/cylc/flow/scripts/graph.py @@ -30,18 +30,20 @@ # display the difference between the flows one and two $ cylc graph one --diff two + + # render the graph to a svg file + $ cylc graph one -o 'one.svg' """ from difflib import unified_diff -import re from shutil import which from subprocess import Popen, PIPE import sys from tempfile import NamedTemporaryFile -from typing import List, Optional, TYPE_CHECKING, Tuple +from typing import Dict, List, Optional, TYPE_CHECKING, Tuple from cylc.flow.config import WorkflowConfig -from cylc.flow.exceptions import InputError +from cylc.flow.exceptions import InputError, CylcError from cylc.flow.id import Tokens from cylc.flow.id_cli import parse_id from cylc.flow.option_parsers import ( @@ -96,22 +98,47 @@ def sort_datetime_edge(item): return (item[0], item[1] or '') -def graph_workflow( +# node/edge types +Node = str # node ID +Edge = Tuple[str, str] # left, right + + +def get_nodes_and_edges( + opts, + workflow_id, + start, + stop, +) -> Tuple[List[Node], List[Edge]]: + """Return graph sorted nodes and edges.""" + config = get_config(workflow_id, opts) + if opts.namespaces: + nodes, edges = _get_inheritance_nodes_and_edges(config) + else: + nodes, edges = _get_graph_nodes_edges( + config, + start, + stop, + grouping=opts.grouping, + show_suicide=opts.show_suicide, + ) + return nodes, edges + + +def _get_graph_nodes_edges( config, start_point_str=None, stop_point_str=None, grouping=None, show_suicide=False, - write=print -): - """Implement ``cylc-graph --reference``.""" +) -> Tuple[List[Node], List[Edge]]: + """Return nodes and edges for a workflow graph.""" graph = config.get_graph_raw( start_point_str, stop_point_str, grouping ) if not graph: - return + return [], [] # set sort keys based on cycling mode if config.cfg['scheduling']['cycling mode'] == 'integer': @@ -123,55 +150,47 @@ def graph_workflow( node_sort = None # lexicographically sortable edge_sort = sort_datetime_edge - edges = ( - (left, right) - for left, right, _, suicide, _ in graph - if right - if show_suicide or not suicide + # get nodes + nodes = sorted( + { + node + for left, right, _, suicide, _ in graph + for node in (left, right) + if node + if show_suicide or not suicide + }, + key=node_sort, ) - for left, right in sorted(set(edges), key=edge_sort): - write('edge "%s" "%s"' % (left, right)) - - write('graph') - - # print nodes - nodes = ( - node - for left, right, _, suicide, _ in graph - for node in (left, right) - if node - if show_suicide or not suicide + + # get edges + edges = sorted( + { + (left, right) + for left, right, _, suicide, _ in graph + if right + if show_suicide or not suicide + }, + key=edge_sort, ) - for node in sorted(set(nodes), key=node_sort): - tokens = Tokens(node, relative=True) - write( - f'node "{node}" "{tokens["task"]}\\n{tokens["cycle"]}"' - ) - write('stop') + return nodes, edges -def graph_inheritance(config, write=print): - """Implement ``cylc-graph --reference --namespaces``.""" - edges = set() +def _get_inheritance_nodes_and_edges( + config +) -> Tuple[List[Node], List[Edge]]: + """Return nodes and edges for an inheritance graph.""" nodes = set() + for namespace in config.get_parent_lists(): + nodes.add(namespace) + + edges = set() for namespace, tasks in config.get_parent_lists().items(): for task in tasks: edges.add((task, namespace)) nodes.add(task) - for namespace in config.get_parent_lists(): - nodes.add(namespace) - - for edge in sorted(edges): - write('edge "%s" "%s"' % edge) - - write('graph') - - for node in sorted(nodes): - write('node "%s" "%s"' % (node, node)) - - write('stop') + return sorted(nodes), sorted(edges) def get_config(workflow_id: str, opts: 'Values') -> WorkflowConfig: @@ -187,6 +206,221 @@ def get_config(workflow_id: str, opts: 'Values') -> WorkflowConfig: ) +def format_graphviz( + opts, + nodes: List[Node], + edges: List[Edge], +) -> List[str]: + """Write graph in graphviz format.""" + # write graph header + dot_lines = [ + 'digraph {', + ' graph [fontname="sans" fontsize="25"]', + ' node [fontname="sans"]', + ] + if opts.transpose: + dot_lines.append(' rankdir="LR"') + if opts.namespaces: + dot_lines.append(' node [shape="rect"]') + dot_lines.append('') + + # write nodes + if opts.namespaces: + dot_lines.extend([ + rf' "{node}"' + for node in nodes + ]) + dot_lines.append('') + else: + # group by cycle + cycles: Dict[str, List[str]] = {} + for node in nodes: + tokens = Tokens(node, relative=True) + cycle: str = tokens['cycle'] + task: str = tokens['task'] + cycles.setdefault(cycle, []).append(task) + # write nodes by cycle + if opts.cycles: + indent = ' ' + else: + indent = ' ' + for cycle, tasks in cycles.items(): + if opts.cycles: + dot_lines.extend( + [ + f' subgraph "cluster_{cycle}" {{ ', + f' label="{cycle}"', + ' style="dashed"', + ] + ) + dot_lines.extend( + rf'{indent}"{cycle}/{task}" [label="{task}\n{cycle}"]' + for task in tasks + ) + if opts.cycles: + dot_lines.append(' }') + dot_lines.append('') + + # write edges + for left, right in edges: + dot_lines.append(f' "{left}" -> "{right}"') + + # close graph + dot_lines.append('}') + + return dot_lines + + +def format_cylc_reference( + opts, + nodes: List[Node], + edges: List[Edge], +) -> List[str]: + """Write graph in cylc reference format.""" + lines = [] + # write edges + for left, right in edges: + lines.append(f'edge "{left}" "{right}"') + + # write separator + lines.append('graph') + + # write nodes + if opts.namespaces: + for node in nodes: + lines.append(f'node "{node}" "{node}"') + else: + for node in nodes: + tokens = Tokens(node, relative=True) + lines.append( + f'node "{node}" "{tokens["task"]}\\n{tokens["cycle"]}"' + ) + + # write terminator + lines.append('stop') + + return lines + + +def render_dot(dot_lines, filename, fmt): + """Render graph using `dot`.""" + # check graphviz-dot is installed + if not which('dot'): + sys.exit('Graphviz must be installed to render graphs.') + + # render graph with graphviz + proc = Popen( # nosec + ['dot', f'-T{fmt}', '-o', filename], + stdin=PIPE, + text=True + ) + proc.communicate('\n'.join(dot_lines)) + proc.wait() + if proc.returncode: + raise CylcError('Graphing Failed') + + +def open_image(filename): + """Open an image file.""" + print(f'Graph rendered to {filename}') + try: + from PIL import Image + except ModuleNotFoundError: + # dependencies required to display images not present + pass + else: + img = Image.open(filename) + img.show() + + +def graph_render(opts, workflow_id, start, stop) -> int: + """Render the workflow graph to the specified format. + + Graph is rendered to the specified format. The Graphviz "dot" format + does not require Graphviz to be installed. + + All other formats require Graphviz. Supported formats depend on your + Graphviz installation. + """ + # get nodes and edges + nodes, edges = get_nodes_and_edges( + opts, + workflow_id, + start, + stop, + ) + + # format the graph in graphviz-dot format + dot_lines = format_graphviz(opts, nodes, edges) + + # set filename and output format + if opts.output: + filename = opts.output + try: + fmt = filename.rsplit('.', 1)[1] + except IndexError: + sys.exit('Output filename requires a format.') + else: + filename = NamedTemporaryFile().name + fmt = 'png' + + if fmt == 'dot': + # output in dot format (graphviz not needed for this) + with open(filename, 'w+') as dot_file: + dot_file.write('\n'.join(dot_lines) + '\n') + return 0 + + # render with graphviz + render_dot(dot_lines, filename, fmt) + + # notify the user / open the graph + if opts.output: + print(f'Graph rendered to {opts.output}') + else: + open_image(filename) + return 0 + + +def graph_reference(opts, workflow_id, start, stop, write=print) -> int: + """Format the workflow graph using the cylc reference format.""" + # get nodes and edges + nodes, edges = get_nodes_and_edges( + opts, + workflow_id, + start, + stop, + ) + for line in format_cylc_reference(opts, nodes, edges): + write(line) + + return 0 + + +def graph_diff(opts, workflow_a, workflow_b, start, stop) -> int: + """Difference the workflow graphs using the cylc reference format.""" + # load graphs + graph_a: List[str] = [] + graph_b: List[str] = [] + graph_reference(opts, workflow_a, start, stop, write=graph_a.append), + graph_reference(opts, workflow_b, start, stop, write=graph_b.append), + + # compare graphs + diff_lines = list( + unified_diff( + [f'{line}\n' for line in graph_a], + [f'{line}\n' for line in graph_b], + fromfile=workflow_a, + tofile=workflow_b, + ) + ) + + # return results + if diff_lines: + sys.stdout.writelines(diff_lines) + return 1 + return 0 + + def get_option_parser() -> COP: """CLI.""" parser = COP( @@ -228,7 +462,7 @@ def get_option_parser() -> COP: '-o', help=( 'Output the graph to a file. The file extension determines the' - ' format.' + ' format. E.G. "graph.png", "graph.svg", "graph.dot".' ), action='store', dest='output' @@ -264,110 +498,6 @@ def get_option_parser() -> COP: return parser -def dot(opts, lines): - """Render a graph using graphviz 'dot'. - - This crudely re-parses the output of the reference output for simplicity. - - This functionality will be replaced by the GUI. - - """ - if not which('dot'): - sys.exit('Graphviz must be installed to render graphs.') - - # set filename and output format - if opts.output: - filename = opts.output - try: - fmt = filename.rsplit('.', 1)[1] - except IndexError: - sys.exit('Output filename requires a format.') - else: - filename = NamedTemporaryFile().name - fmt = 'png' - - # scrape nodes and edges from the reference output - node = re.compile(r'node "(.*)" "(.*)"') - edge = re.compile(r'edge "(.*)" "(.*)"') - nodes = {} - edges = [] - for line in lines: - match = node.match(line) - if match: - if opts.namespaces: - task = match.group(1) - cycle = '' - else: - cycle, task = match.group(1).split('/') - nodes.setdefault(cycle, []).append(task) - continue - match = edge.match(line) - if match: - edges.append(match.groups()) - - # write graph header - dot = [ - 'digraph {', - ' graph [fontname="sans" fontsize="25"]', - ' node [fontname="sans"]', - ] - if opts.transpose: - dot.append(' rankdir="LR"') - if opts.namespaces: - dot.append(' node [shape="rect"]') - - # write nodes - for cycle, tasks in nodes.items(): - if opts.cycles: - dot.extend( - [ - f' subgraph "cluster_{cycle}" {{ ', - f' label="{cycle}"', - ' style="dashed"', - ] - ) - dot.extend( - rf' "{cycle}/{task}" [label="{task}\n{cycle}"]' - for task in tasks - ) - dot.append(' }' if opts.cycles else '') - - # write edges - for left, right in edges: - dot.append(f' "{left}" -> "{right}"') - - # close graph - dot.append('}') - - # render graph - proc = Popen( # nosec - ['dot', f'-T{fmt}', '-o', filename], - stdin=PIPE, - text=True - ) - # * filename is generated in code above - # * fmt is user specified and quoted (by subprocess) - proc.communicate('\n'.join(dot)) - proc.wait() - if proc.returncode: - sys.exit('Graphing Failed') - - return filename - - -def gui(filename): - """Open the rendered image file.""" - print(f'Graph rendered to {filename}') - try: - from PIL import Image - except ModuleNotFoundError: - # dependencies required to display images not present - pass - else: - img = Image.open(filename) - img.show() - - @cli_function(get_option_parser) def main( parser: COP, @@ -379,50 +509,17 @@ def main( """Implement ``cylc graph``.""" if opts.grouping and opts.namespaces: raise InputError('Cannot combine --group and --namespaces.') - - lines: List[str] = [] - if not (opts.reference or opts.diff): - write = lines.append - else: - write = print - - flows: List[Tuple[str, List[str]]] = [(workflow_id, [])] - if opts.diff: - flows.append((opts.diff, [])) - - for flow, graph in flows: - if opts.diff: - write = graph.append - config = get_config(flow, opts) - if opts.namespaces: - graph_inheritance(config, write=write) - else: - graph_workflow( - config, - start, - stop, - grouping=opts.grouping, - show_suicide=opts.show_suicide, - write=write - ) + if opts.cycles and opts.namespaces: + raise InputError('Cannot combine --cycles and --namespaces.') if opts.diff: - diff_lines = list( - unified_diff( - [f'{line}\n' for line in flows[0][1]], - [f'{line}\n' for line in flows[1][1]], - fromfile=flows[0][0], - tofile=flows[1][0] - ) + sys.exit( + graph_diff(opts, workflow_id, opts.diff, start, stop) ) - - if diff_lines: - sys.stdout.writelines(diff_lines) - sys.exit(1) - - if not (opts.reference or opts.diff): - filename = dot(opts, lines) - if opts.output: - print(f'Graph rendered to {opts.output}') - else: - gui(filename) + if opts.reference: + sys.exit( + graph_reference(opts, workflow_id, start, stop) + ) + sys.exit( + graph_render(opts, workflow_id, start, stop) + ) From b3d60996e9e96bd093866651aafba8c80d680b22 Mon Sep 17 00:00:00 2001 From: Oliver Sanders Date: Mon, 20 Jun 2022 11:46:56 +0100 Subject: [PATCH 21/66] graph: test output formats * test `cylc graph` output formats. --- cylc/flow/scripts/graph.py | 5 +- tests/unit/scripts/test_graph.py | 305 +++++++++++++++++++++++++++++++ 2 files changed, 309 insertions(+), 1 deletion(-) mode change 100755 => 100644 cylc/flow/scripts/graph.py create mode 100644 tests/unit/scripts/test_graph.py diff --git a/cylc/flow/scripts/graph.py b/cylc/flow/scripts/graph.py old mode 100755 new mode 100644 index aa0c8dda48b..7e73861094a --- a/cylc/flow/scripts/graph.py +++ b/cylc/flow/scripts/graph.py @@ -40,7 +40,7 @@ from subprocess import Popen, PIPE import sys from tempfile import NamedTemporaryFile -from typing import Dict, List, Optional, TYPE_CHECKING, Tuple +from typing import Dict, List, Optional, TYPE_CHECKING, Tuple, Callable from cylc.flow.config import WorkflowConfig from cylc.flow.exceptions import InputError, CylcError @@ -141,6 +141,9 @@ def _get_graph_nodes_edges( return [], [] # set sort keys based on cycling mode + # (note sorting is very important for the reference format) + node_sort: Optional[Callable] + edge_sort: Optional[Callable] if config.cfg['scheduling']['cycling mode'] == 'integer': # integer sorting node_sort = sort_integer_node diff --git a/tests/unit/scripts/test_graph.py b/tests/unit/scripts/test_graph.py new file mode 100644 index 00000000000..876ddb2600f --- /dev/null +++ b/tests/unit/scripts/test_graph.py @@ -0,0 +1,305 @@ +# THIS FILE IS PART OF THE CYLC WORKFLOW ENGINE. +# Copyright (C) NIWA & British Crown (Met Office) & Contributors. +# +# This program is free software: you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation, either version 3 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program. If not, see . + +from types import SimpleNamespace +import typing as t + +import pytest + +from cylc.flow.scripts.graph import ( + Edge, + Node, + format_cylc_reference, + format_graphviz, + get_nodes_and_edges, +) + + +@pytest.fixture +def example_graph(): + """Example workflow graph with inter-cycle dependencies.""" + nodes: t.List[Node] = [ + '1/a', + '1/b', + '1/c', + '2/a', + '2/b', + '2/c', + ] + edges: t.List[Edge] = [ + ('1/a', '1/b'), + ('1/b', '1/c'), + ('2/a', '2/b'), + ('2/b', '2/c'), + ('1/b', '2/b'), + ] + return nodes, edges + + +@pytest.fixture +def example_namespace_graph(): + """Example namespace graph with inheritance.""" + nodes: t.List[Node] = [ + 'A', + 'a1', + 'a2', + 'B', + 'B1', + 'b11', + 'B2', + 'b22', + ] + edges: t.List[Edge] = [ + ('A', 'a1'), + ('A', 'a2'), + ('B', 'B1'), + ('B', 'B2'), + ('B1', 'b11'), + ('B2', 'b22'), + ] + return nodes, edges + + +@pytest.fixture +def null_config(monkeypatch): + """Path the config loader to return a workflow with no nodes or edges.""" + def _get_graph_raw(*args, **kwargs): + return None + + def _get_parents_lists(*args, **kwargs): + return {} + + config = SimpleNamespace( + get_graph_raw=_get_graph_raw, + get_parent_lists=_get_parents_lists, + ) + + monkeypatch.setattr( + 'cylc.flow.scripts.graph.get_config', + lambda x, y: config + ) + + +def test_format_graphviz_normal(example_graph): + """Test graphviz output for default options. + + Tests both orientations (--transpose). + """ + nodes, edges = example_graph + + # format the graph in regular orientation + opts = SimpleNamespace(transpose=False, namespaces=False, cycles=False) + lines = format_graphviz(opts, nodes, edges) + assert lines == [ + 'digraph {', + ' graph [fontname="sans" fontsize="25"]', + ' node [fontname="sans"]', + '', + ' "1/a" [label="a\\n1"]', + ' "1/b" [label="b\\n1"]', + ' "1/c" [label="c\\n1"]', + '', + ' "2/a" [label="a\\n2"]', + ' "2/b" [label="b\\n2"]', + ' "2/c" [label="c\\n2"]', + '', + ' "1/a" -> "1/b"', + ' "1/b" -> "1/c"', + ' "2/a" -> "2/b"', + ' "2/b" -> "2/c"', + ' "1/b" -> "2/b"', + '}', + ] + + # format the graph in transposed orientation + opts = SimpleNamespace(transpose=True, namespaces=False, cycles=False) + transposed_lines = format_graphviz(opts, nodes, edges) + + # the transposed graph should be the same except for one line... + assert [ + line + for line in transposed_lines + if line not in lines + ] == [ + # ...the one which sets the orientation + ' rankdir="LR"', + ] + + +def test_format_graphviz_namespace(example_namespace_graph): + """Test graphviz output for a namespace graph. + + Tests both orientations (--transpose). + """ + nodes, edges = example_namespace_graph + + # format the graph in regular orientation + opts = SimpleNamespace(transpose=False, namespaces=True, cycles=False) + lines = format_graphviz(opts, nodes, edges) + assert lines == [ + 'digraph {', + ' graph [fontname="sans" fontsize="25"]', + ' node [fontname="sans"]', + ' node [shape="rect"]', + '', + ' "A"', + ' "a1"', + ' "a2"', + ' "B"', + ' "B1"', + ' "b11"', + ' "B2"', + ' "b22"', + '', + ' "A" -> "a1"', + ' "A" -> "a2"', + ' "B" -> "B1"', + ' "B" -> "B2"', + ' "B1" -> "b11"', + ' "B2" -> "b22"', + '}', + ] + + # format the graph in transposed orientation + opts = SimpleNamespace(transpose=True, namespaces=True, cycles=False) + transposed_lines = format_graphviz(opts, nodes, edges) + + # the transposed graph should be the same except for one line... + assert [ + line + for line in transposed_lines + if line not in lines + ] == [ + # ...the one which sets the orientation + ' rankdir="LR"', + ] + + +def test_format_graphviz_cycles(example_graph): + """Test graphviz format for the --cycles option (group by cycle). + + Note: There is no difference between iso8601 and integer cycle points here, + the graph logic is cycle point format agnostic. Sorting is not performed + in this funtion. + """ + nodes, edges = example_graph + + opts = SimpleNamespace(transpose=False, namespaces=False, cycles=True) + lines = format_graphviz(opts, nodes, edges) + assert lines == [ + 'digraph {', + ' graph [fontname="sans" fontsize="25"]', + ' node [fontname="sans"]', + '', + ' subgraph "cluster_1" { ', + ' label="1"', + ' style="dashed"', + ' "1/a" [label="a\\n1"]', + ' "1/b" [label="b\\n1"]', + ' "1/c" [label="c\\n1"]', + ' }', + '', + ' subgraph "cluster_2" { ', + ' label="2"', + ' style="dashed"', + ' "2/a" [label="a\\n2"]', + ' "2/b" [label="b\\n2"]', + ' "2/c" [label="c\\n2"]', + ' }', + '', + ' "1/a" -> "1/b"', + ' "1/b" -> "1/c"', + ' "2/a" -> "2/b"', + ' "2/b" -> "2/c"', + ' "1/b" -> "2/b"', + '}', + ] + + +def test_format_cylc_reference_normal(example_graph): + """Test Cylc "reference" format (used by the test battery). + + Note: There is no difference between iso8601 and integer cycle points here, + the graph logic is cycle point format agnostic. Sorting is not performed + in this funtion. + + Note: There is no transpose mode for reference graphs. + """ + nodes, edges = example_graph + + opts = SimpleNamespace(namespaces=False) + lines = format_cylc_reference(opts, nodes, edges) + assert lines == [ + 'edge "1/a" "1/b"', + 'edge "1/b" "1/c"', + 'edge "2/a" "2/b"', + 'edge "2/b" "2/c"', + 'edge "1/b" "2/b"', + 'graph', + 'node "1/a" "a\\n1"', + 'node "1/b" "b\\n1"', + 'node "1/c" "c\\n1"', + 'node "2/a" "a\\n2"', + 'node "2/b" "b\\n2"', + 'node "2/c" "c\\n2"', + 'stop', + ] + + +def test_format_cylc_reference_namespace(example_namespace_graph): + """Test Cylc "reference" format for namespace graphs. + + Note: There is no transpose mode for reference graphs. + """ + nodes, edges = example_namespace_graph + + opts = SimpleNamespace(namespaces=True) + lines = format_cylc_reference(opts, nodes, edges) + assert lines == [ + 'edge "A" "a1"', + 'edge "A" "a2"', + 'edge "B" "B1"', + 'edge "B" "B2"', + 'edge "B1" "b11"', + 'edge "B2" "b22"', + 'graph', + 'node "A" "A"', + 'node "a1" "a1"', + 'node "a2" "a2"', + 'node "B" "B"', + 'node "B1" "B1"', + 'node "b11" "b11"', + 'node "B2" "B2"', + 'node "b22" "b22"', + 'stop', + ] + + +def test_null(null_config): + """Ensure that an empty graph is handled elegantly.""" + opts = SimpleNamespace( + namespaces=False, + grouping=False, + show_suicide=False + ) + assert get_nodes_and_edges(opts, None, 1, 2) == ([], []) + + opts = SimpleNamespace( + namespaces=True, + grouping=False, + show_suicide=False + ) + assert get_nodes_and_edges(opts, None, 1, 2) == ([], []) From f05474898ece8d913b483b8c29ea4a8a9839be50 Mon Sep 17 00:00:00 2001 From: Oliver Sanders Date: Thu, 4 Aug 2022 14:50:07 +0100 Subject: [PATCH 22/66] cat-log: test log colourisation --- cylc/flow/scripts/cat_log.py | 9 ++- tests/unit/scripts/test_cat_log.py | 89 ++++++++++++++++++++++++++++++ 2 files changed, 95 insertions(+), 3 deletions(-) create mode 100644 tests/unit/scripts/test_cat_log.py diff --git a/cylc/flow/scripts/cat_log.py b/cylc/flow/scripts/cat_log.py index 1a1c9bd05ab..2767bacde40 100755 --- a/cylc/flow/scripts/cat_log.py +++ b/cylc/flow/scripts/cat_log.py @@ -124,7 +124,7 @@ BUFSIZE = 1024 * 1024 -def colorise_cat_log(cat_proc, color=False): +def colorise_cat_log(cat_proc, color=False, stdout=None): """Print a Cylc log file in color at it would appear in the terminal. Args: @@ -133,6 +133,8 @@ def colorise_cat_log(cat_proc, color=False): color (bool): If `True` log will appear in color, if `False` no control characters will be added. + stdout: + Set the stdout argument of "Popen" if "color=True". """ if color: @@ -146,9 +148,10 @@ def colorise_cat_log(cat_proc, color=False): ]), # * there is no untrusted input, everything is hardcoded ], - stdin=PIPE + stdin=PIPE, + stdout=stdout, ) - color_proc.communicate(cat_proc.communicate()[0]) + return color_proc.communicate(cat_proc.communicate()[0]) else: cat_proc.wait() diff --git a/tests/unit/scripts/test_cat_log.py b/tests/unit/scripts/test_cat_log.py new file mode 100644 index 00000000000..37a179e1292 --- /dev/null +++ b/tests/unit/scripts/test_cat_log.py @@ -0,0 +1,89 @@ +# THIS FILE IS PART OF THE CYLC WORKFLOW ENGINE. +# Copyright (C) NIWA & British Crown (Met Office) & Contributors. +# +# This program is free software: you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation, either version 3 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program. If not, see . + +from subprocess import Popen, PIPE + +from ansimarkup import parse as cparse +from colorama import Style +import pytest + +from cylc.flow.loggingutil import CylcLogFormatter +from cylc.flow.scripts.cat_log import ( + colorise_cat_log, +) + + +@pytest.fixture +def log_file(tmp_path): + _log_file = tmp_path / 'log' + with open(_log_file, 'w+') as fh: + fh.write('DEBUG - 1\n') + fh.write('INFO - 2\n') + fh.write('WARNING - 3\n') + fh.write('ERROR - 4\n') + fh.write('CRITICAL - 5\n') + return _log_file + + +def test_colorise_cat_log_plain(log_file): + """It should not colourise logs when color=False.""" + # command for colorise_cat_log to colourise + cat_proc = Popen( + ['cat', str(log_file)], + stdout=PIPE, + ) + colorise_cat_log(cat_proc, color=False) + assert cat_proc.communicate()[0].decode().splitlines() == [ + # there should not be any ansii color characters here + 'DEBUG - 1', + 'INFO - 2', + 'WARNING - 3', + 'ERROR - 4', + 'CRITICAL - 5', + ] + + +def test_colorise_cat_log_colour(log_file): + """It should colourise logs when color=True.""" + # command for colorise_cat_log to colourise + cat_proc = Popen( + ['cat', str(log_file)], + stdout=PIPE, + ) + out, err = colorise_cat_log(cat_proc, color=True, stdout=PIPE) + + # strip the line breaks (because tags can come before or after them) + # strip the reset tags (because they might not be needed if redeclared) + out = ''.join( + line.replace(Style.RESET_ALL, '') + for line in out.decode().splitlines() + ) + + col = CylcLogFormatter.COLORS + assert out == ( + ''.join([ + # strip the reset tags + cparse(line).replace(Style.RESET_ALL, '') + for line in [ + col['DEBUG'].format('DEBUG - 1'), + 'INFO - 2', + col['WARNING'].format('WARNING - 3'), + col['ERROR'].format('ERROR - 4'), + col['CRITICAL'].format('CRITICAL - 5'), + '' + ] + ]) + ) From b84dd82bd3b577f3622fd078ef17c6a8f6f03427 Mon Sep 17 00:00:00 2001 From: Oliver Sanders Date: Thu, 4 Aug 2022 14:57:42 +0100 Subject: [PATCH 23/66] cli: add tests for `cylc help` options --- tests/functional/cli/01-help.t | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/tests/functional/cli/01-help.t b/tests/functional/cli/01-help.t index 5e1db5f7cc8..3f78ada0dbf 100755 --- a/tests/functional/cli/01-help.t +++ b/tests/functional/cli/01-help.t @@ -19,11 +19,14 @@ . "$(dirname "$0")/test_header" # Number of tests depends on the number of 'cylc' commands. -set_test_number 23 +set_test_number 26 # Top help run_ok "${TEST_NAME_BASE}-0" cylc run_ok "${TEST_NAME_BASE}-help" cylc help +run_ok "${TEST_NAME_BASE}-help-id" cylc help id +run_ok "${TEST_NAME_BASE}-help-all" cylc help all +grep_ok 'trigger ...' "${TEST_NAME_BASE}-help-all.stdout" run_ok "${TEST_NAME_BASE}---help" cylc --help for FILE in \ "${TEST_NAME_BASE}-help.stdout" \ From 0b649b556f9f7e68bfbb269460357127c7716de3 Mon Sep 17 00:00:00 2001 From: Oliver Sanders Date: Thu, 4 Aug 2022 16:43:40 +0100 Subject: [PATCH 24/66] show: add integration tests for meta queries --- cylc/flow/scripts/show.py | 107 ++++++++++++----- tests/integration/scripts/test_show.py | 153 +++++++++++++++++++++++++ 2 files changed, 229 insertions(+), 31 deletions(-) create mode 100644 tests/integration/scripts/test_show.py diff --git a/cylc/flow/scripts/show.py b/cylc/flow/scripts/show.py index 3fa4c6cf83d..00721f9cf62 100755 --- a/cylc/flow/scripts/show.py +++ b/cylc/flow/scripts/show.py @@ -36,6 +36,7 @@ workflow database. """ +import asyncio import json from optparse import Values import sys @@ -52,8 +53,8 @@ TASK_STATUS_RUNNING ) from cylc.flow.option_parsers import ( + CylcOptionParser as COP, ID_MULTI_ARG_DOC, - CylcOptionParser as COP ) from cylc.flow.terminal import cli_function @@ -144,6 +145,31 @@ def print_msg_state(msg, state): def flatten_data(data, flat_data=None): + """Reduce a nested data structure to a flat one. + + Examples: + It flattens out nested dictionaries: + >>> flatten_data({}) + {} + >>> flatten_data({'a': 1}) + {'a': 1} + >>> flatten_data({'a': {'b': 2, 'c': {'d': 4}}}) + {'b': 2, 'd': 4} + + It iterates through any lists that it finds: + >>> flatten_data({'a': [{'b': 2}, {'c': 3}]}) + {'b': 2, 'c': 3} + + Overriding is determined by iteration order (don't rely on it): + >>> flatten_data({'a': 1, 'b': {'a': 2}}) + {'a': 2} + + It can't flatten things which aren't dicts: + >>> flatten_data({'a': ['x', 'y']}) + Traceback (most recent call last): + AttributeError: 'str' object has no attribute 'items' + + """ if flat_data is None: flat_data = {} for key, value in data.items(): @@ -191,14 +217,14 @@ def get_option_parser(): return parser -def workflow_meta_query(workflow_id, pclient, options, json_filter): +async def workflow_meta_query(workflow_id, pclient, options, json_filter): query = WORKFLOW_META_QUERY query_kwargs = { 'request_string': query, 'variables': {'wFlows': [workflow_id]} } # Print workflow info. - results = pclient('graphql', query_kwargs) + results = await pclient.async_request('graphql', query_kwargs) for workflow_id in results['workflows']: flat_data = flatten_data(workflow_id) if options.json: @@ -211,7 +237,7 @@ def workflow_meta_query(workflow_id, pclient, options, json_filter): return 0 -def prereqs_and_outputs_query( +async def prereqs_and_outputs_query( workflow_id, tokens_list, pclient, @@ -232,7 +258,7 @@ def prereqs_and_outputs_query( 'taskIds': ids_list, } } - results = pclient('graphql', tp_kwargs) + results = await pclient.async_request('graphql', tp_kwargs) multi = len(results['taskProxies']) > 1 for t_proxy in results['taskProxies']: task_id = Tokens(t_proxy['id']).relative_id @@ -332,7 +358,13 @@ def prereqs_and_outputs_query( return 0 -def task_meta_query(workflow_id, task_names, pclient, options, json_filter): +async def task_meta_query( + workflow_id, + task_names, + pclient, + options, + json_filter, +): tasks_query = TASK_META_QUERY tasks_kwargs = { 'request_string': tasks_query, @@ -342,7 +374,7 @@ def task_meta_query(workflow_id, task_names, pclient, options, json_filter): }, } # Print workflow info. - results = pclient('graphql', tasks_kwargs) + results = await pclient.async_request('graphql', tasks_kwargs) multi = len(results['tasks']) > 1 for task in results['tasks']: flat_data = flatten_data(task['meta']) @@ -357,46 +389,59 @@ def task_meta_query(workflow_id, task_names, pclient, options, json_filter): return 0 -@cli_function(get_option_parser) -def main(_, options: 'Values', *ids) -> None: - """Implement "cylc show" CLI.""" - workflow_args, _ = parse_ids( - *ids, - constraint='mixed', - max_workflows=1, +async def show(workflow_id, tokens_list, opts): + pclient = get_client( + workflow_id, + timeout=opts.comms_timeout, ) - workflow_id = list(workflow_args)[0] - tokens_list = workflow_args[workflow_id] - - if tokens_list and options.task_defs: - raise InputError( - 'Cannot query both live tasks and task definitions.' - ) - - pclient = get_client(workflow_id, timeout=options.comms_timeout) json_filter: 'Dict' = {} ret = 0 - if options.task_defs: - ret = task_meta_query( + if opts.task_defs: + ret = await task_meta_query( workflow_id, - options.task_defs, + opts.task_defs, pclient, - options, + opts, json_filter, ) elif not tokens_list: - ret = workflow_meta_query(workflow_id, pclient, options, json_filter) + ret = await workflow_meta_query( + workflow_id, + pclient, + opts, + json_filter, + ) else: - ret = prereqs_and_outputs_query( + ret = await prereqs_and_outputs_query( workflow_id, tokens_list, pclient, - options, + opts, json_filter, ) - if options.json: + if opts.json: print(json.dumps(json_filter, indent=4)) + return ret + + +@cli_function(get_option_parser) +def main(_, options: 'Values', *ids) -> None: + """Implement "cylc show" CLI.""" + workflow_args, _ = parse_ids( + *ids, + constraint='mixed', + max_workflows=1, + ) + workflow_id = list(workflow_args)[0] + tokens_list = workflow_args[workflow_id] + + if tokens_list and options.task_defs: + raise InputError( + 'Cannot query both live tasks and task definitions.' + ) + + ret = asyncio.run(show(workflow_id, tokens_list, options)) sys.exit(ret) diff --git a/tests/integration/scripts/test_show.py b/tests/integration/scripts/test_show.py new file mode 100644 index 00000000000..4efc0739747 --- /dev/null +++ b/tests/integration/scripts/test_show.py @@ -0,0 +1,153 @@ +# THIS FILE IS PART OF THE CYLC WORKFLOW ENGINE. +# Copyright (C) NIWA & British Crown (Met Office) & Contributors. +# +# This program is free software: you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation, either version 3 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program. If not, see . + +import json +import pytest +from types import SimpleNamespace + +from colorama import init as colour_init + +from cylc.flow.id import Tokens +from cylc.flow.scripts.show import ( + show, +) + + +@pytest.fixture(scope='module') +def mod_my_conf(): + """A workflow configuration with some workflow metadata.""" + return { + 'meta': { + 'title': 'Workflow Title', + 'description': """ + My + multiline + description. + """, + 'URL': 'http://ismycomputerturnedon.com/', + 'answer': '42', + }, + 'scheduling': { + 'graph': { + 'R1': 'foo' + } + }, + 'runtime': { + 'foo': { + 'meta': { + 'title': 'Task Title', + 'description': ''' + Task + multiline + description + ''', + 'URL': ( + 'http://hasthelargehadroncollider' + 'destroyedtheworldyet.com/' + ), + 'question': 'mutually exclusive', + } + } + }, + } + + +@pytest.fixture(scope='module') +async def mod_my_schd(mod_flow, mod_scheduler, mod_start, mod_my_conf): + """A "started" workflow.""" + id_ = mod_flow(mod_my_conf) + schd = mod_scheduler(id_) + async with mod_start(schd): + yield schd + + +async def test_workflow_meta_query(mod_my_schd, capsys): + """It should fetch workflow metadata.""" + colour_init(strip=True, autoreset=True) + opts = SimpleNamespace( + comms_timeout=5, + json=False, + list_prereqs=False, + task_defs=None, + ) + + # plain output + ret = await show(mod_my_schd.workflow, [], opts) + assert ret == 0 + out, err = capsys.readouterr() + assert out.splitlines() == [ + 'title: Workflow Title', + 'description: My', + 'multiline', + 'description.', + 'answer: 42', + 'URL: http://ismycomputerturnedon.com/', + ] + + # json output + opts.json = True + ret = await show(mod_my_schd.workflow, [], opts) + assert ret == 0 + out, err = capsys.readouterr() + assert json.loads(out) == { + 'title': 'Workflow Title', + 'description': 'My\nmultiline\ndescription.', + 'answer': '42', + 'URL': 'http://ismycomputerturnedon.com/', + } + + +async def test_task_meta_query(mod_my_schd, capsys): + """It should fetch task metadata.""" + colour_init(strip=True, autoreset=True) + opts = SimpleNamespace( + comms_timeout=5, + json=False, + list_prereqs=False, + task_defs=['foo'], + ) + + # plain output + ret = await show( + mod_my_schd.workflow, + # [Tokens(cycle='1', task='foo')], + None, + opts, + ) + assert ret == 0 + out, err = capsys.readouterr() + assert out.splitlines() == [ + 'title: Task Title', + 'question: mutually exclusive', + 'description: Task', + 'multiline', + 'description', + 'URL: http://hasthelargehadroncolliderdestroyedtheworldyet.com/', + ] + + # json output + opts.json = True + ret = await show(mod_my_schd.workflow, [], opts) + assert ret == 0 + out, err = capsys.readouterr() + assert json.loads(out) == { + 'foo': { + 'title': 'Task Title', + 'question': 'mutually exclusive', + 'description': 'Task\nmultiline\ndescription', + 'URL': 'http://hasthelargehadroncolliderdestroyedtheworldyet.com/', + } + } From f440533fccfdd12bfe76b547c4604a61c1441d9d Mon Sep 17 00:00:00 2001 From: Oliver Sanders Date: Mon, 15 Aug 2022 11:30:08 +0100 Subject: [PATCH 25/66] config: make regexes stricter and test * Clock and external trigger patterns allowed partial matches meaning that invalid clock or external triggers could pass through. * Test matching of trigger patterns and fill some coverage holes. --- cylc/flow/config.py | 95 ++++++++++++---- tests/integration/conftest.py | 4 +- tests/integration/test_config.py | 183 +++++++++++++++++++++++++++++++ 3 files changed, 260 insertions(+), 22 deletions(-) diff --git a/cylc/flow/config.py b/cylc/flow/config.py index 685dcfe7f9f..b52c8fcdb59 100644 --- a/cylc/flow/config.py +++ b/cylc/flow/config.py @@ -36,7 +36,7 @@ import traceback from typing import ( Any, Callable, Dict, List, Mapping, Optional, Set, TYPE_CHECKING, Tuple, - Union + Union, Iterable ) from metomi.isodatetime.data import Calendar @@ -114,18 +114,44 @@ from cylc.flow.cycling import IntervalBase, PointBase, SequenceBase -RE_CLOCK_OFFSET = re.compile(r'(' + TaskID.NAME_RE + r')(?:\(\s*(.+)\s*\))?') -RE_EXT_TRIGGER = re.compile(r'(.*)\s*\(\s*(.+)\s*\)\s*') +RE_CLOCK_OFFSET = re.compile( + rf''' + ^ + \s* + ({TaskID.NAME_RE}) # task name + (?:\(\s*(.+)\s*\))? # optional (arguments, ...) + \s* + $ + ''', + re.X, +) +RE_EXT_TRIGGER = re.compile( + r''' + ^ + \s* + (.*) # task name + \s* + \(\s*(.+)\s*\) # required (arguments, ...) + \s* + $ + ''', + re.X, +) RE_SEC_MULTI_SEQ = re.compile(r'(?![^(]+\)),') RE_WORKFLOW_ID_VAR = re.compile(r'\${?CYLC_WORKFLOW_(REG_)?ID}?') RE_TASK_NAME_VAR = re.compile(r'\${?CYLC_TASK_NAME}?') RE_VARNAME = re.compile(r'^[a-zA-Z_][\w]*$') -def check_varnames(env): +def check_varnames(env: Iterable[str]) -> List[str]: """Check a list of env var names for legality. Return a list of bad names (empty implies success). + + Examples: + >>> check_varnames(['foo', 'BAR', '+baz']) + ['+baz'] + """ bad = [] for varname in env: @@ -142,6 +168,23 @@ def interpolate_template(tmpl, params_dict): If it fails, raises ParamExpandError, but if the string does not contain `%(`, it just returns the string. + + Examples: + >>> interpolate_template('_%(a)s_', {'a': 'A'}) + '_A_' + + >>> interpolate_template('%(a)s', {'b': 'B'}) + Traceback (most recent call last): + cylc.flow.exceptions.ParamExpandError: bad parameter + + >>> interpolate_template('%(a)d', {'a': 'A'}) + Traceback (most recent call last): + cylc.flow.exceptions.ParamExpandError: wrong data type for parameter + + >>> interpolate_template('%(as', {}) + Traceback (most recent call last): + cylc.flow.exceptions.ParamExpandError: bad template syntax + """ if '%(' not in tmpl: return tmpl # User probably not trying to use param template @@ -155,7 +198,25 @@ def interpolate_template(tmpl, params_dict): raise ParamExpandError('bad template syntax') -# TODO: separate config for run and non-run purposes? +def dequote(string): + """Strip quotes off a string. + + Examples: + >>> dequote('"foo"') + 'foo' + >>> dequote("'foo'") + 'foo' + >>> dequote('foo') + 'foo' + >>> dequote('"f') + '"f' + + """ + if len(string) < 2: + return string + if (string[0] == string[-1]) and string.startswith(("'", '"')): + return string[1:-1] + return string class WorkflowConfig: @@ -450,7 +511,7 @@ def __init__( elif s_type == 'clock-expire': self.expiration_offsets[name] = offset_interval elif s_type == 'external-trigger': - self.ext_triggers[name] = self.dequote(ext_trigger_msg) + self.ext_triggers[name] = dequote(ext_trigger_msg) self.cfg['scheduling']['special tasks'][s_type] = result @@ -988,13 +1049,6 @@ def validate_namespace_names(self): f'task/family name {message}\n[runtime][[{name}]]' ) - @staticmethod - def dequote(s): - """Strip quotes off a string.""" - if (s[0] == s[-1]) and s.startswith(("'", '"')): - return s[1:-1] - return s - def check_env_names(self): """Check for illegal environment variable names""" bad = {} @@ -1005,14 +1059,15 @@ def check_env_names(self): if res: bad[(label, key)] = res if bad: - err_msg = "bad env variable names:" - for (label, key), names in bad.items(): - err_msg += '\nNamespace:\t%s [%s]' % (label, key) - for name in names: - err_msg += "\n\t\t%s" % name - LOG.error(err_msg) raise WorkflowConfigError( - "Illegal environment variable name(s) detected") + "Illegal environment variable name(s) detected:\n* " + # f"\n{err_msg}" + + '\n* '.join( + f'[runtime][{label}][{key}]{name}' + for (label, key), names in bad.items() + for name in names + ) + ) def check_param_env_tmpls(self): """Check for illegal parameter environment templates""" diff --git a/tests/integration/conftest.py b/tests/integration/conftest.py index 23179a73f42..6a6ec3275a6 100644 --- a/tests/integration/conftest.py +++ b/tests/integration/conftest.py @@ -338,8 +338,8 @@ def validate(run_dir): reg - The flow to validate kwargs - Arguments to pass to ValidateOptions """ - def _validate(reg: str, **kwargs) -> None: - WorkflowConfig( + def _validate(reg: str, **kwargs) -> WorkflowConfig: + return WorkflowConfig( reg, str(Path(run_dir, reg, 'flow.cylc')), ValidateOptions(**kwargs) diff --git a/tests/integration/test_config.py b/tests/integration/test_config.py index 082e133fe0d..453075cc581 100644 --- a/tests/integration/test_config.py +++ b/tests/integration/test_config.py @@ -99,3 +99,186 @@ def test_validate_implicit_task_name( assert str(exc_ctx.value).splitlines()[0] == ( f'invalid task name "{task_name}"' ) + + +@pytest.mark.parametrize( + 'env_var,valid', [ + ('foo', True), + ('FOO', True), + ('+foo', False), + ] +) +def test_validate_env_vars(flow, one_conf, validate, env_var, valid): + """It should validate environment variable names.""" + reg = flow({ + **one_conf, + 'runtime': { + 'foo': { + 'environment': { + env_var: 'value' + } + } + } + }) + if valid: + validate(reg) + else: + with pytest.raises(WorkflowConfigError) as exc_ctx: + validate(reg) + assert env_var in str(exc_ctx.value) + + +@pytest.mark.parametrize( + 'env_val', [ + '%(x)s', # valid template but no such parameter x + '%(a)123', # invalid template + ] +) +def test_validate_param_env_templ( + flow, + one_conf, + validate, + env_val, + caplog, + log_filter, +): + """It should validate parameter environment templates.""" + reg = flow({ + **one_conf, + 'runtime': { + 'foo': { + 'environment': { + 'foo': env_val + } + } + } + }) + validate(reg) + assert log_filter(caplog, contains='bad parameter environment template') + assert log_filter(caplog, contains=env_val) + + +def test_no_graph(flow, validate): + """It should fail for missing graph sections.""" + reg = flow({ + 'scheduling': {}, + }) + with pytest.raises(WorkflowConfigError) as exc_ctx: + validate(reg) + assert 'missing [scheduling][[graph]] section.' in str(exc_ctx.value) + + +def test_parameter_templates_setting(flow, one_conf, validate): + """It should fail if [task parameter]templates is a setting. + + It should be a section. + """ + reg = flow({ + **one_conf, + 'task parameters': { + 'templates': 'foo' + } + }) + with pytest.raises(WorkflowConfigError) as exc_ctx: + validate(reg) + assert '[templates] is a section' in str(exc_ctx.value) + + +@pytest.mark.parametrize( + 'section', [ + 'external-trigger', + 'clock-trigger', + 'clock-expire', + ] +) +def test_parse_special_tasks_invalid(flow, validate, section): + """It should fail for invalid "special tasks".""" + reg = flow({ + 'scheduler': { + 'allow implicit tasks': 'True', + }, + 'scheduling': { + 'initial cycle point': 'now', + 'special tasks': { + section: 'foo (', # missing closing bracket + }, + 'graph': { + 'R1': 'foo', + }, + } + }) + with pytest.raises(WorkflowConfigError) as exc_ctx: + validate(reg) + assert f'Illegal {section} spec' in str(exc_ctx.value) + assert 'foo' in str(exc_ctx.value) + + +def test_parse_special_tasks_interval(flow, validate): + """It should fail for invalid durations in clock-triggers.""" + reg = flow({ + 'scheduler': { + 'allow implicit tasks': 'True', + }, + 'scheduling': { + 'initial cycle point': 'now', + 'special tasks': { + 'clock-trigger': 'foo(PT1Y)', # invalid ISO8601 duration + }, + 'graph': { + 'R1': 'foo' + } + } + }) + with pytest.raises(WorkflowConfigError) as exc_ctx: + validate(reg) + assert 'Illegal clock-trigger spec' in str(exc_ctx.value) + assert 'PT1Y' in str(exc_ctx.value) + + +@pytest.mark.parametrize( + 'section', [ + 'external-trigger', + 'clock-trigger', + 'clock-expire', + ] +) +def test_parse_special_tasks_families(flow, scheduler, validate, section): + """It should expand families in special tasks.""" + reg = flow({ + 'scheduling': { + 'initial cycle point': 'now', + 'special tasks': { + section: 'FOO(P1D)', + }, + 'graph': { + 'R1': 'foo & foot', + } + }, + 'runtime': { + # family + 'FOO': {}, + # nested family + 'FOOT': { + 'inherit': 'FOO', + }, + 'foo': { + 'inherit': 'FOO', + }, + 'foot': { + 'inherit': 'FOOT', + }, + } + }) + if section == 'external-trigger': + # external triggers cannot be used for multiple tasks so if family + # expansion is completed correctly, validation should fail + with pytest.raises(WorkflowConfigError) as exc_ctx: + config = validate(reg) + assert 'external triggers must be used only once' in str(exc_ctx.value) + else: + config = validate(reg) + assert config.cfg['scheduling']['special tasks'][section] == [ + # the family FOO has been expanded to the tasks foo, foot + 'foo(P1D)', + 'foot(P1D)' + ] From 31a158f7081e46fc20fff63b8ad283a647517479 Mon Sep 17 00:00:00 2001 From: Oliver Sanders Date: Mon, 15 Aug 2022 15:14:40 +0100 Subject: [PATCH 26/66] Apply suggestions from code review Co-authored-by: Ronnie Dutta <61982285+MetRonnie@users.noreply.github.com> --- cylc/flow/scripts/graph.py | 4 +--- tests/integration/scripts/test_show.py | 1 - tests/unit/scripts/test_graph.py | 2 +- 3 files changed, 2 insertions(+), 5 deletions(-) diff --git a/cylc/flow/scripts/graph.py b/cylc/flow/scripts/graph.py index 7e73861094a..3488177ac17 100644 --- a/cylc/flow/scripts/graph.py +++ b/cylc/flow/scripts/graph.py @@ -184,11 +184,9 @@ def _get_inheritance_nodes_and_edges( ) -> Tuple[List[Node], List[Edge]]: """Return nodes and edges for an inheritance graph.""" nodes = set() - for namespace in config.get_parent_lists(): - nodes.add(namespace) - edges = set() for namespace, tasks in config.get_parent_lists().items(): + nodes.add(namespace) for task in tasks: edges.add((task, namespace)) nodes.add(task) diff --git a/tests/integration/scripts/test_show.py b/tests/integration/scripts/test_show.py index 4efc0739747..e1a8280d97b 100644 --- a/tests/integration/scripts/test_show.py +++ b/tests/integration/scripts/test_show.py @@ -123,7 +123,6 @@ async def test_task_meta_query(mod_my_schd, capsys): # plain output ret = await show( mod_my_schd.workflow, - # [Tokens(cycle='1', task='foo')], None, opts, ) diff --git a/tests/unit/scripts/test_graph.py b/tests/unit/scripts/test_graph.py index 876ddb2600f..aa64002fca7 100644 --- a/tests/unit/scripts/test_graph.py +++ b/tests/unit/scripts/test_graph.py @@ -75,7 +75,7 @@ def example_namespace_graph(): @pytest.fixture def null_config(monkeypatch): - """Path the config loader to return a workflow with no nodes or edges.""" + """Patch the config loader to return a workflow with no nodes or edges.""" def _get_graph_raw(*args, **kwargs): return None From 00b9f2e71c9b0995cae87a6be0774adcee3d1711 Mon Sep 17 00:00:00 2001 From: Oliver Sanders Date: Tue, 16 Aug 2022 09:31:01 +0100 Subject: [PATCH 27/66] scan: ignore FileNotFoundError (#5065) * During a scan directories are listed asynchronously and recursively. * If a directory is queued for listing, but then deleted before the listing takes place a FileNotFoundError will occur. * This is highly unlikely IRL, however, in the integration tests, where run dirs are created and destroyed at scale and in parallel this is actually quite likely. --- cylc/flow/network/scan.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/cylc/flow/network/scan.py b/cylc/flow/network/scan.py index 201ee4f14fc..7c8116b1a70 100644 --- a/cylc/flow/network/scan.py +++ b/cylc/flow/network/scan.py @@ -253,7 +253,12 @@ def _scan_subdirs(listing: List[Path], depth: int) -> None: return_when=asyncio.FIRST_COMPLETED ) for task in done: - path, depth, contents = task.result() + try: + path, depth, contents = task.result() + except FileNotFoundError: + # directory has been removed since the scan was scheduled + running.remove(task) + continue running.remove(task) is_flow = dir_is_flow(contents) if is_flow: From 6e84663a6626956f5423d1a66a1420074c8b86a2 Mon Sep 17 00:00:00 2001 From: Tim Pillinger <26465611+wxtim@users.noreply.github.com> Date: Tue, 16 Aug 2022 11:45:40 +0100 Subject: [PATCH 28/66] Lint.hardcode style index numbers (#5055) * Hardcoded error numbers for the style checks to prevent error number drift in future Added an --ignore/-n command to exclude style rules. added testing for ignore * Update cylc/flow/scripts/lint.py Co-authored-by: Ronnie Dutta <61982285+MetRonnie@users.noreply.github.com> Co-authored-by: Ronnie Dutta <61982285+MetRonnie@users.noreply.github.com> --- CHANGES.md | 4 +++ cylc/flow/scripts/lint.py | 56 ++++++++++++++++++++++++--------- tests/unit/scripts/test_lint.py | 30 ++++++++++++++---- 3 files changed, 69 insertions(+), 21 deletions(-) diff --git a/CHANGES.md b/CHANGES.md index ab60946d252..8349f99a2fd 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -37,6 +37,10 @@ ones in. --> [#5032](https://github.com/cylc/cylc-flow/pull/5032) - set a default limit of 100 for the "default" queue. +[#5055](https://github.com/cylc/cylc-flow/pull/5055) - Hard-code the serial +numbers of Cylc Lint's style issues and allow users to ignore Cylc Lint issues +using `--ignore `. + ------------------------------------------------------------------------------- ## __cylc-8.0.1 (Upcoming)__ diff --git a/cylc/flow/scripts/lint.py b/cylc/flow/scripts/lint.py index a1d917f7f43..aeffadc94ab 100755 --- a/cylc/flow/scripts/lint.py +++ b/cylc/flow/scripts/lint.py @@ -83,17 +83,20 @@ STYLE_CHECKS = { re.compile(r'^\t'): { 'short': 'Use multiple spaces, not tabs', - 'url': STYLE_GUIDE + 'tab-characters' + 'url': STYLE_GUIDE + 'tab-characters', + 'index': 1 }, # Not a full test, but if a non section is not indented... re.compile(r'^[^\{\[|\s]'): { 'short': 'Item not indented.', - 'url': STYLE_GUIDE + 'indentation' + 'url': STYLE_GUIDE + 'indentation', + 'index': 2 }, # [section] re.compile(r'^\s+\[[^\[.]*\]'): { 'short': 'Top level sections should not be indented.', - 'url': STYLE_GUIDE + 'indentation' + 'url': STYLE_GUIDE + 'indentation', + 'index': 3 }, # 2 or 4 space indentation both seem reasonable: re.compile(r'^(|\s|\s{2,3}|\s{5,})\[\[[^\[.]*\]\]'): { @@ -101,28 +104,32 @@ 'Second level sections should be indented exactly ' '4 spaces.' ), - 'url': STYLE_GUIDE + 'indentation' + 'url': STYLE_GUIDE + 'indentation', + 'index': 4 }, re.compile(r'^(|\s{1,7}|\s{9,})\[\[\[[^\[.]*\]\]\]'): { 'short': ( 'Third level sections should be indented exactly ' '8 spaces.' ), - 'url': STYLE_GUIDE + 'indentation' + 'url': STYLE_GUIDE + 'indentation', + 'index': 5 }, re.compile(r'\s$'): { 'short': 'trailing whitespace.', - 'url': STYLE_GUIDE + 'trailing-whitespace' + 'url': STYLE_GUIDE + 'trailing-whitespace', + 'index': 6 + }, + re.compile(r'inherit\s*=\s*[a-z].*$'): { + 'short': 'Family name contains lowercase characters.', + 'url': STYLE_GUIDE + 'task-naming-conventions', + 'index': 7 }, # Consider re-adding this as an option later: # re.compile(r'^.{80,}'): { # 'short': 'line > 79 characters.', - # 'url': STYLE_GUIDE + 'line-length-and-continuation' + # 'url': STYLE_GUIDE + 'line-length-and-continuation', # }, - re.compile(r'inherit\s*=\s*[a-z].*$'): { - 'short': 'Family name contains lowercase characters.', - 'url': STYLE_GUIDE + 'task-naming-conventions' - }, } # Subset of deprecations which are tricky (impossible?) to scrape from the # upgrader. @@ -257,9 +264,15 @@ def get_checkset_from_name(name): return purpose_filters -def parse_checks(check_arg): +def parse_checks(check_arg, ignores=None): """Collapse metadata in checks dicts. + + Args: + check_arg: type of checks to run, currently expecting '728', 'style' + or 'all'. + ignores: list of codes to ignore. """ + ignores = ignores or [] parsedchecks = {} purpose_filters = get_checkset_from_name(check_arg) @@ -269,8 +282,10 @@ def parse_checks(check_arg): if purpose in purpose_filters: for index, (pattern, meta) in enumerate(ruleset.items(), start=1): meta.update({'purpose': purpose}) - meta.update({'index': index}) - parsedchecks.update({pattern: meta}) + if 'index' not in meta: + meta.update({'index': index}) + if f'{purpose}{index:03d}' not in ignores: + parsedchecks.update({pattern: meta}) return parsedchecks @@ -435,6 +450,17 @@ def get_option_parser() -> COP: default=False, dest='ref_mode' ) + parser.add_option( + '--ignore', '-n', + help=( + 'Ignore this check number.' + ), + action='append', + default=[], + dest='ignores', + metavar="CODE", + choices=tuple([f'S{i["index"]:03d}' for i in STYLE_CHECKS.values()]) + ) return parser @@ -481,7 +507,7 @@ def main(parser: COP, options: 'Values', *targets) -> None: check_names = options.linter # Check each file: - checks = parse_checks(check_names) + checks = parse_checks(check_names, options.ignores) for file_ in get_cylc_files(target): LOG.debug(f'Checking {file_}') count += check_cylc_file( diff --git a/tests/unit/scripts/test_lint.py b/tests/unit/scripts/test_lint.py index 560c716024f..2ad27e98c88 100644 --- a/tests/unit/scripts/test_lint.py +++ b/tests/unit/scripts/test_lint.py @@ -17,6 +17,7 @@ """Tests `cylc lint` CLI Utility. """ import difflib +from itertools import combinations from pathlib import Path import pytest import re @@ -148,9 +149,9 @@ @pytest.fixture() def create_testable_file(monkeypatch, capsys): - def _inner(test_file, checks): + def _inner(test_file, checks, ignores=[]): monkeypatch.setattr(Path, 'read_text', lambda _: test_file) - checks = parse_checks(checks) + checks = parse_checks(checks, ignores) check_cylc_file(Path('x'), Path('x'), checks) return capsys.readouterr(), checks return _inner @@ -190,15 +191,32 @@ def test_check_cylc_file_line_no(create_testable_file, capsys): def test_check_cylc_file_lint(create_testable_file, number): try: result, _ = create_testable_file( - LINT_TEST_FILE, 'lint') - assert f'[S{number + 1:03d}]' in result.out + LINT_TEST_FILE, 'style') + assert f'S{(number + 1):03d}' in result.out except AssertionError: raise AssertionError( - f'missing error number S:{number:03d}' - f'{[*STYLE_CHECKS.keys()][number]}' + f'missing error number S{number:03d}:' + f'{[*STYLE_CHECKS.keys()][number].pattern}' ) +@pytest.mark.parametrize( + 'exclusion', + [ + comb for i in range(len(STYLE_CHECKS.values())) + for comb in combinations( + [f'S{i["index"]:03d}' for i in STYLE_CHECKS.values()], i + 1 + ) + ] +) +def test_check_exclusions(create_testable_file, exclusion): + """It does not report any items excluded.""" + result, _ = create_testable_file( + LINT_TEST_FILE, 'style', list(exclusion)) + for item in exclusion: + assert item not in result.out + + @pytest.fixture def create_testable_dir(tmp_path): test_file = (tmp_path / 'suite.rc') From af64ace2f078b7d656c83c09db483a531f43b98f Mon Sep 17 00:00:00 2001 From: Ronnie Dutta <61982285+MetRonnie@users.noreply.github.com> Date: Tue, 16 Aug 2022 16:19:34 +0100 Subject: [PATCH 29/66] Add workflow field to ClientError, ClientTimeout Plus 1 or 2 type annotations --- cylc/flow/exceptions.py | 18 ++++++++++++++++-- cylc/flow/network/__init__.py | 9 +++++---- cylc/flow/network/client.py | 2 +- 3 files changed, 22 insertions(+), 7 deletions(-) diff --git a/cylc/flow/exceptions.py b/cylc/flow/exceptions.py index 89791cf0940..67e2dfa6104 100644 --- a/cylc/flow/exceptions.py +++ b/cylc/flow/exceptions.py @@ -254,9 +254,17 @@ def __str__(self): class ClientError(CylcError): - def __init__(self, message: str, traceback: Optional[str] = None): + def __init__( + self, + message: str, + traceback: Optional[str] = None, + workflow: Optional[str] = None + ): self.message = message self.traceback = traceback + # Workflow not included in string representation but useful bit of + # info to attach to the exception object + self.workflow = workflow def __str__(self) -> str: ret = self.message @@ -277,7 +285,13 @@ def __str__(self): class ClientTimeout(CylcError): - pass + + def __init__(self, message: str, workflow: Optional[str] = None): + self.message = message + self.workflow = workflow + + def __str__(self) -> str: + return self.message class CyclingError(CylcError): diff --git a/cylc/flow/network/__init__.py b/cylc/flow/network/__init__.py index e12b1d5ef61..5614638002b 100644 --- a/cylc/flow/network/__init__.py +++ b/cylc/flow/network/__init__.py @@ -18,6 +18,7 @@ import asyncio import getpass import json +from typing import Tuple import zmq import zmq.asyncio @@ -59,17 +60,17 @@ def decode_(message): return msg -def get_location(workflow: str): +def get_location(workflow: str) -> Tuple[str, int, int]: """Extract host and port from a workflow's contact file. NB: if it fails to load the workflow contact file, it will exit. Args: - workflow (str): workflow name + workflow: workflow name Returns: - Tuple[str, int, int]: tuple with the host name and port numbers. + tuple with the host name and port numbers. Raises: - ClientError: if the workflow is not running. + WorkflowStopped: if the workflow is not running. CylcVersionError: if target is a Cylc 7 (or earlier) workflow. """ try: diff --git a/cylc/flow/network/client.py b/cylc/flow/network/client.py index ae917c41aa9..cc70233fc17 100644 --- a/cylc/flow/network/client.py +++ b/cylc/flow/network/client.py @@ -101,7 +101,7 @@ class WorkflowRuntimeClient(ZMQSocketBase): "args": {...}} Raises: - ClientError: if the workflow is not running. + WorkflowStopped: if the workflow is not running. Call server "endpoints" using: ``__call__``, ``serial_request`` From 9ea493276ade4b93fb33e5a0af738f5edce3c1af Mon Sep 17 00:00:00 2001 From: Oliver Sanders Date: Tue, 16 Aug 2022 16:32:48 +0100 Subject: [PATCH 30/66] bump dev version --- cylc/flow/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cylc/flow/__init__.py b/cylc/flow/__init__.py index d1e69dccbd8..1227bf0a407 100644 --- a/cylc/flow/__init__.py +++ b/cylc/flow/__init__.py @@ -46,7 +46,7 @@ def environ_init(): environ_init() -__version__ = '8.0.2.dev' +__version__ = '8.1.0.dev' def iter_entry_points(entry_point_name): From 312a9de2aa6be8f23702e307050d6e18255e9f23 Mon Sep 17 00:00:00 2001 From: Oliver Sanders Date: Wed, 17 Aug 2022 15:39:53 +0100 Subject: [PATCH 31/66] tui: add poll mutation (#5075) --- cylc/flow/tui/data.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/cylc/flow/tui/data.py b/cylc/flow/tui/data.py index 95068f93217..0a304bec46f 100644 --- a/cylc/flow/tui/data.py +++ b/cylc/flow/tui/data.py @@ -92,13 +92,15 @@ 'hold', 'release', 'kill', - 'trigger' + 'trigger', + 'poll', ], 'task': [ 'hold', 'release', 'kill', - 'trigger' + 'trigger', + 'poll', ], 'job': [ 'kill', From fe793e31652846d55daafebbc625ef37ee8eacf1 Mon Sep 17 00:00:00 2001 From: Ronnie Dutta <61982285+MetRonnie@users.noreply.github.com> Date: Thu, 18 Aug 2022 10:50:49 +0100 Subject: [PATCH 32/66] Fix FileNotFoundError when scanning path that doesn't exist --- cylc/flow/network/scan.py | 5 +++- tests/functional/cylc-clean/00-basic.t | 12 ++++++-- tests/integration/test_async_util.py | 41 -------------------------- tests/integration/test_scan.py | 7 +++++ tests/unit/test_async_util.py | 24 ++++++++++++++- 5 files changed, 43 insertions(+), 46 deletions(-) delete mode 100644 tests/integration/test_async_util.py diff --git a/cylc/flow/network/scan.py b/cylc/flow/network/scan.py index 7c8116b1a70..c929c2079a7 100644 --- a/cylc/flow/network/scan.py +++ b/cylc/flow/network/scan.py @@ -238,7 +238,10 @@ def _scan_subdirs(listing: List[Path], depth: int) -> None: ) # perform the first directory listing - scan_dir_listing = await scandir(scan_dir) + try: + scan_dir_listing = await scandir(scan_dir) + except FileNotFoundError: + return if scan_dir != cylc_run_dir and dir_is_flow(scan_dir_listing): # If the scan_dir itself is a workflow run dir, yield nothing return diff --git a/tests/functional/cylc-clean/00-basic.t b/tests/functional/cylc-clean/00-basic.t index f7fd40c1700..1f4cec84984 100644 --- a/tests/functional/cylc-clean/00-basic.t +++ b/tests/functional/cylc-clean/00-basic.t @@ -21,7 +21,7 @@ if ! command -v 'tree' >'/dev/null'; then skip_all '"tree" command not available' fi -set_test_number 8 +set_test_number 10 # Generate random name for symlink dirs to avoid any clashes with other tests SYM_NAME="$(mktemp -u)" @@ -94,7 +94,7 @@ ${TEST_DIR}/${SYM_NAME}/work/cylc-run/${CYLC_TEST_REG_BASE} \`-- work __TREE__ # ----------------------------------------------------------------------------- -TEST_NAME="cylc-clean" +TEST_NAME="clean" run_ok "$TEST_NAME" cylc clean "$WORKFLOW_NAME" dump_std "$TEST_NAME" # ----------------------------------------------------------------------------- @@ -109,5 +109,11 @@ ${TEST_DIR}/${SYM_NAME}/log/cylc-run/${CYLC_TEST_REG_BASE} \`-- leave-me-alone __TREE__ # ----------------------------------------------------------------------------- +TEST_NAME="clean-non-exist" +run_ok "$TEST_NAME" cylc clean "$WORKFLOW_NAME" +dump_std "$TEST_NAME" +cmp_ok "${TEST_NAME}.stdout" << __EOF__ +INFO - No directory to clean at ${WORKFLOW_RUN_DIR} +__EOF__ +# ----------------------------------------------------------------------------- purge -exit diff --git a/tests/integration/test_async_util.py b/tests/integration/test_async_util.py deleted file mode 100644 index 6c9bfaa565e..00000000000 --- a/tests/integration/test_async_util.py +++ /dev/null @@ -1,41 +0,0 @@ -# THIS FILE IS PART OF THE CYLC WORKFLOW ENGINE. -# Copyright (C) NIWA & British Crown (Met Office) & Contributors. -# -# This program is free software: you can redistribute it and/or modify -# it under the terms of the GNU General Public License as published by -# the Free Software Foundation, either version 3 of the License, or -# (at your option) any later version. -# -# This program is distributed in the hope that it will be useful, -# but WITHOUT ANY WARRANTY; without even the implied warranty of -# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the -# GNU General Public License for more details. -# -# You should have received a copy of the GNU General Public License -# along with this program. If not, see . - -from pathlib import Path -from shutil import rmtree - -import pytest - -from cylc.flow.async_util import scandir - - -@pytest.fixture() -def directory(tmp_path): - """A directory with two files and a symlink.""" - (tmp_path / 'a').touch() - (tmp_path / 'b').touch() - (tmp_path / 'c').symlink_to(tmp_path / 'b') - yield tmp_path - rmtree(tmp_path) - - -async def test_scandir(directory): - """It should list directory contents (including symlinks).""" - assert sorted(await scandir(directory)) == [ - Path(directory, 'a'), - Path(directory, 'b'), - Path(directory, 'c') - ] diff --git a/tests/integration/test_scan.py b/tests/integration/test_scan.py index cef762c394e..94edbfc9b29 100644 --- a/tests/integration/test_scan.py +++ b/tests/integration/test_scan.py @@ -250,6 +250,13 @@ async def test_scan_nasty_symlinks(run_dir_with_nasty_symlinks): ] +async def test_scan_non_exist(tmp_path: Path): + """Calling scan() on a scan_dir that doesn't exist should not raise.""" + assert await listify( + scan(scan_dir=(tmp_path / 'HORSE')) + ) == [] + + async def test_is_active(sample_run_dir): """It should filter flows by presence of a contact file.""" # running flows diff --git a/tests/unit/test_async_util.py b/tests/unit/test_async_util.py index 13cda504b48..17823817dbe 100644 --- a/tests/unit/test_async_util.py +++ b/tests/unit/test_async_util.py @@ -16,13 +16,15 @@ import asyncio import logging +from pathlib import Path from random import random import pytest from cylc.flow.async_util import ( pipe, - asyncqgen + asyncqgen, + scandir, ) LOG = logging.getLogger('test') @@ -238,3 +240,23 @@ async def test_asyncqgen(): ret.append(item) assert ret == [1, 2, 3] + + +async def test_scandir(tmp_path: Path): + """It should list directory contents (including symlinks).""" + (tmp_path / 'a').touch() + (tmp_path / 'b').touch() + (tmp_path / 'c').symlink_to(tmp_path / 'b') + + assert sorted(await scandir(tmp_path)) == [ + Path(tmp_path, 'a'), + Path(tmp_path, 'b'), + Path(tmp_path, 'c') + ] + + +async def test_scandir_non_exist(tmp_path: Path): + """scandir() should raise FileNotFoundError if called on a path that + doesn't exist.""" + with pytest.raises(FileNotFoundError): + await scandir(tmp_path / 'HORSE') From 53f13aa47438b5eed29eb29a81b71e2c00f91b02 Mon Sep 17 00:00:00 2001 From: Oliver Sanders Date: Thu, 18 Aug 2022 14:56:12 +0100 Subject: [PATCH 33/66] terms: settle on "task" and "job" but not "task job" * Retire "task job" as a user-facing term in favour of the canonical "task" and "job" terms. * Closes https://github.com/cylc/cylc-doc/issues/352 * The ID structure is `workflow//cycle/task/job`, they are tasks and jobs not "task-jobs" or "cycle-tasks" or "workflow-cycles". --- INSTALL.md | 2 +- cylc/flow/cfgspec/globalcfg.py | 8 +++--- cylc/flow/cfgspec/workflow.py | 28 +++++++++---------- cylc/flow/etc/job.sh | 10 +++---- cylc/flow/hostuserutil.py | 2 +- cylc/flow/job_file.py | 6 ++-- cylc/flow/job_runner_handlers/at.py | 2 +- cylc/flow/job_runner_handlers/background.py | 2 +- cylc/flow/job_runner_handlers/loadleveler.py | 4 +-- cylc/flow/job_runner_handlers/lsf.py | 4 +-- cylc/flow/job_runner_handlers/moab.py | 4 +-- cylc/flow/job_runner_handlers/pbs.py | 4 +-- cylc/flow/job_runner_handlers/sge.py | 4 +-- cylc/flow/job_runner_handlers/slurm.py | 4 +-- cylc/flow/job_runner_mgr.py | 6 ++-- cylc/flow/network/schema.py | 20 ++++++------- cylc/flow/scripts/cat_log.py | 4 +-- cylc/flow/scripts/jobs_submit.py | 2 +- cylc/flow/scripts/message.py | 14 +++++----- cylc/flow/scripts/pause.py | 2 +- cylc/flow/scripts/poll.py | 2 +- cylc/flow/task_events_mgr.py | 2 +- cylc/flow/task_job_logs.py | 4 +-- cylc/flow/task_job_mgr.py | 14 +++++----- cylc/flow/task_message.py | 4 +-- cylc/flow/task_pool.py | 4 +-- cylc/flow/task_remote_mgr.py | 2 +- .../restart/41-auto-restart-local-jobs.t | 2 +- tests/functional/shutdown/09-now2.t | 2 +- tests/functional/shutdown/22-stop-now.t | 2 +- tests/integration/test_scheduler.py | 2 +- tests/unit/test_job_file.py | 2 +- 32 files changed, 87 insertions(+), 87 deletions(-) diff --git a/INSTALL.md b/INSTALL.md index ab7a2d177f3..ffe0166ed25 100644 --- a/INSTALL.md +++ b/INSTALL.md @@ -5,7 +5,7 @@ **See the [Cylc installation guide](https://cylc.github.io/cylc-doc/stable/html/installation.html) for more detailed information.** -Cylc must be installed on workflow and task job hosts. Third-party dependencies +Cylc must be installed on workflow and job hosts. Third-party dependencies are not required on job hosts. ## Cylc 7 diff --git a/cylc/flow/cfgspec/globalcfg.py b/cylc/flow/cfgspec/globalcfg.py index 567ebf7dd72..e637434e8b0 100644 --- a/cylc/flow/cfgspec/globalcfg.py +++ b/cylc/flow/cfgspec/globalcfg.py @@ -355,7 +355,7 @@ - slurm_packjob - moab -Directives are written to the top of the task job script in the correct format +Directives are written to the top of the job script in the correct format for the job runner. Specifying directives individually like this allows use of default directives @@ -1082,7 +1082,7 @@ def default_for( A platform consists of a group of one or more hosts which share a file system and a job runner (batch system). - A platform must allow interaction with the same task job from *any* + A platform must allow interaction with the same job from *any* of its hosts. .. versionadded:: 8.0.0 @@ -1535,7 +1535,7 @@ def default_for( We recommend using a clean job submission environment for consistent handling of local and remote jobs. However, this is not the default behavior because it prevents - local task jobs from running, unless ``$PATH`` contains the + local jobs from running, unless ``$PATH`` contains the ``cylc`` wrapper script. Specific environment variables can be singled out to pass @@ -1655,7 +1655,7 @@ def default_for( all be suitable for a given job. - When Cylc sets up a task job it will pick a platform from a group. + When Cylc submits a job it will pick a platform from a group. Cylc will then use the selected platform for all interactions with that job. diff --git a/cylc/flow/cfgspec/workflow.py b/cylc/flow/cfgspec/workflow.py index f242adb0c9a..3be8214ce35 100644 --- a/cylc/flow/cfgspec/workflow.py +++ b/cylc/flow/cfgspec/workflow.py @@ -970,7 +970,7 @@ def get_script_common_text(this: str, example: Optional[str] = None): If no parents are listed default is ``root``. ''') Conf('script', VDR.V_STRING, desc=dedent(''' - The main custom script run from the task job script. + The main custom script run from the job script. It can be an external command or script, or inlined scripting. @@ -980,7 +980,7 @@ def get_script_common_text(this: str, example: Optional[str] = None): this='script', example='my_script.sh' )) Conf('init-script', VDR.V_STRING, desc=dedent(''' - Custom script run by the task job script before the task + Custom script run by the job script before the task execution environment is configured. By running before the task execution environment is configured, @@ -992,7 +992,7 @@ def get_script_common_text(this: str, example: Optional[str] = None): this should no longer be necessary. ''') + get_script_common_text(this='init-script')) Conf('env-script', VDR.V_STRING, desc=dedent(''' - Custom script run by the task job script between the + Custom script run by the job script between the cylc-defined environment (workflow and task identity, etc.) and the user-defined task runtime environment. @@ -1001,10 +1001,10 @@ def get_script_common_text(this: str, example: Optional[str] = None): It can be an external command or script, or inlined scripting. ''') + get_script_common_text(this='env-script')) Conf('err-script', VDR.V_STRING, desc=(''' - Script run when a task job error is detected. + Script run when a job error is detected. Custom script to be run at the end of the error trap, - which is triggered due to failure of a command in the task job + which is triggered due to failure of a command in the job script or trappable job kill. The output of this script will always @@ -1029,7 +1029,7 @@ def get_script_common_text(this: str, example: Optional[str] = None): this='exit-script', example='rm -f "$TMP_FILES"' )) Conf('pre-script', VDR.V_STRING, desc=dedent(''' - Custom script run by the task job script immediately + Custom script run by the job script immediately before :cylc:conf:`[..]script`. The pre-script can be an external command or script, or @@ -1039,7 +1039,7 @@ def get_script_common_text(this: str, example: Optional[str] = None): example='echo "Hello from workflow ${CYLC_WORKFLOW_ID}!"' )) Conf('post-script', VDR.V_STRING, desc=dedent(''' - Custom script run by the task job script immediately + Custom script run by the job script immediately after :cylc:conf:`[..]script`. The post-script can be an external @@ -1049,7 +1049,7 @@ def get_script_common_text(this: str, example: Optional[str] = None): Conf('work sub-directory', VDR.V_STRING, desc=''' The directory from which tasks are executed. - Task job scripts are executed from within *work directories* + Job scripts are executed from within *work directories* created automatically under the workflow run directory. A task can get its own work directory from ``$CYLC_TASK_WORK_DIR`` (or ``$PWD`` if it does not ``cd`` elsewhere at @@ -1083,13 +1083,13 @@ def get_script_common_text(this: str, example: Optional[str] = None): ) ) Conf('execution retry delays', VDR.V_INTERVAL_LIST, None, desc=f''' - Cylc can automate resubmission of a failed task job. + Cylc can automate resubmission of a failed job. Execution retry delays are a list of ISO 8601 durations/intervals which tell Cylc how long to wait before resubmitting a failed job. - Each time Cylc resubmits a task job it will increment the + Each time Cylc resubmits a job it will increment the variable ``$CYLC_TASK_TRY_NUMBER`` in the task execution environment. ``$CYLC_TASK_TRY_NUMBER`` allows you to vary task behavior between submission attempts. @@ -1101,13 +1101,13 @@ def get_script_common_text(this: str, example: Optional[str] = None): ''') Conf('execution time limit', VDR.V_INTERVAL, desc=f''' Set the execution (:term:`wallclock `) time - limit of a task job. + limit of a job. For ``background`` and ``at`` job runners Cylc runs the job's script using the timeout command. For other job runners Cylc will convert execution time limit to a :term:`directive`. - If a task job exceeds its execution time limit Cylc can + If a job exceeds its execution time limit Cylc can poll the job multiple times. You can set polling intervals using :cylc:conf:`global.cylc[platforms] []execution time limit polling intervals` @@ -1573,7 +1573,7 @@ def get_script_common_text(this: str, example: Optional[str] = None): The user defined task execution environment. Variables defined here can refer to cylc workflow and task - identity variables, which are exported earlier in the task job + identity variables, which are exported earlier in the job script. Variable assignment expressions can use cylc utility commands because access to cylc is also configured earlier in the script. @@ -1591,7 +1591,7 @@ def get_script_common_text(this: str, example: Optional[str] = None): The order of definition is preserved that each variable can refer to previously defined - variables. Values are passed through to the task job + variables. Values are passed through to the job script without evaluation or manipulation by Cylc (with the exception of valid Python string templates that match parameterized task names - see below), so any diff --git a/cylc/flow/etc/job.sh b/cylc/flow/etc/job.sh index 7b1cda3ad46..3337fa6435d 100644 --- a/cylc/flow/etc/job.sh +++ b/cylc/flow/etc/job.sh @@ -19,11 +19,11 @@ # along with this program. If not, see . ############################################################################### -# Bash functions for a cylc task job. +# Bash functions for a cylc job. ############################################################################### ############################################################################### -# The main function for a cylc task job. +# The main function for a cylc job. cylc__job__main() { # Export CYLC_ workflow and task environment variables cylc__job__inst__cylc_env @@ -50,7 +50,7 @@ cylc__job__main() { trap "cylc__job_vacation ${signal_name}" "${signal_name}" done set -euo pipefail - # Write task job self-identify + # Write job self-identify USER="${USER:-$(whoami)}" typeset host="${HOSTNAME:-}" if [[ -z "${host}" ]]; then @@ -65,7 +65,7 @@ cylc__job__main() { # We were using a HERE document for writing info here until we notice that # Bash uses temporary files for HERE documents, which can be inefficient. echo "Workflow : ${CYLC_WORKFLOW_ID}" - echo "Task Job : ${CYLC_TASK_JOB} (try ${CYLC_TASK_TRY_NUMBER})" + echo "Job : ${CYLC_TASK_JOB} (try ${CYLC_TASK_TRY_NUMBER})" echo "User@Host: ${USER}@${host}" echo # Derived environment variables @@ -229,7 +229,7 @@ cylc__job__poll_grep_workflow_log() { } ############################################################################### -# Run a function in the task job instance file, if possible. +# Run a function in the job instance file, if possible. # Arguments: # func_name: name of function without the "cylc__job__inst__" prefix # Returns: diff --git a/cylc/flow/hostuserutil.py b/cylc/flow/hostuserutil.py index d92c9640e14..41b83b62120 100644 --- a/cylc/flow/hostuserutil.py +++ b/cylc/flow/hostuserutil.py @@ -150,7 +150,7 @@ def get_host(self): As specified by the "[scheduler][host self-identification]" settings in the site/user global.cylc files. This is mainly used for workflow host - identification by task jobs. + identification by tasks. """ if self._host is None: diff --git a/cylc/flow/job_file.py b/cylc/flow/job_file.py index b386c3eb0f2..f6d64159b65 100644 --- a/cylc/flow/job_file.py +++ b/cylc/flow/job_file.py @@ -13,7 +13,7 @@ # # You should have received a copy of the GNU General Public License # along with this program. If not, see . -"""Write task job files.""" +"""Write job files.""" from contextlib import suppress import os @@ -31,7 +31,7 @@ class JobFileWriter: - """Write task job files.""" + """Write job files.""" def __init__(self): self.workflow_env = {} @@ -120,7 +120,7 @@ def _check_script_value(value): def _write_header(handle, job_conf): """Write job script header.""" handle.write("#!/bin/bash -l\n") - handle.write("#\n# ++++ THIS IS A CYLC TASK JOB SCRIPT ++++") + handle.write("#\n# ++++ THIS IS A CYLC JOB SCRIPT ++++") for prefix, value in [ ("# Workflow: ", job_conf['workflow_name']), ("# Task: ", job_conf['task_id']), diff --git a/cylc/flow/job_runner_handlers/at.py b/cylc/flow/job_runner_handlers/at.py index 3ea17a06554..85b3bd6d950 100644 --- a/cylc/flow/job_runner_handlers/at.py +++ b/cylc/flow/job_runner_handlers/at.py @@ -13,7 +13,7 @@ # # You should have received a copy of the GNU General Public License # along with this program. If not, see . -"""Submits task job scripts to the rudimentary Unix ``at`` scheduler. +"""Submits job scripts to the rudimentary Unix ``at`` scheduler. .. cylc-scope:: flow.cylc[runtime][] diff --git a/cylc/flow/job_runner_handlers/background.py b/cylc/flow/job_runner_handlers/background.py index 5dd44f0e1c5..2866a839dfb 100644 --- a/cylc/flow/job_runner_handlers/background.py +++ b/cylc/flow/job_runner_handlers/background.py @@ -13,7 +13,7 @@ # # You should have received a copy of the GNU General Public License # along with this program. If not, see . -"""Runs task job scripts as Unix background processes. +"""Runs job scripts as Unix background processes. .. cylc-scope:: flow.cylc[runtime][] diff --git a/cylc/flow/job_runner_handlers/loadleveler.py b/cylc/flow/job_runner_handlers/loadleveler.py index ae5d6b753cf..ee8203b2b47 100644 --- a/cylc/flow/job_runner_handlers/loadleveler.py +++ b/cylc/flow/job_runner_handlers/loadleveler.py @@ -13,7 +13,7 @@ # # You should have received a copy of the GNU General Public License # along with this program. If not, see . -"""Submits task job scripts to loadleveler by the ``llsubmit`` command. +"""Submits job scripts to loadleveler by the ``llsubmit`` command. .. cylc-scope:: flow.cylc[runtime][] @@ -37,7 +37,7 @@ foo = bar baz = qux -These are written to the top of the task job script like this: +These are written to the top of the job script like this: .. code-block:: bash diff --git a/cylc/flow/job_runner_handlers/lsf.py b/cylc/flow/job_runner_handlers/lsf.py index d1a3323cef4..a465c9b7924 100644 --- a/cylc/flow/job_runner_handlers/lsf.py +++ b/cylc/flow/job_runner_handlers/lsf.py @@ -13,7 +13,7 @@ # # You should have received a copy of the GNU General Public License # along with this program. If not, see . -"""Submits task job scripts to IBM Platform LSF by the ``bsub`` command. +"""Submits job scripts to IBM Platform LSF by the ``bsub`` command. .. cylc-scope:: flow.cylc[runtime][] @@ -36,7 +36,7 @@ [[[directives]]] -q = foo -These are written to the top of the task job script like this: +These are written to the top of the job script like this: .. code-block:: bash diff --git a/cylc/flow/job_runner_handlers/moab.py b/cylc/flow/job_runner_handlers/moab.py index 11365b0dfde..839d246ccbc 100644 --- a/cylc/flow/job_runner_handlers/moab.py +++ b/cylc/flow/job_runner_handlers/moab.py @@ -13,7 +13,7 @@ # # You should have received a copy of the GNU General Public License # along with this program. If not, see . -"""Submits task job scripts to the Moab workload manager with ``msub``. +"""Submits job scripts to the Moab workload manager with ``msub``. .. cylc-scope:: flow.cylc[runtime][] @@ -39,7 +39,7 @@ -q = foo -l nodes = 1 -These are written to the top of the task job script like this: +These are written to the top of the job script like this: .. code-block:: bash diff --git a/cylc/flow/job_runner_handlers/pbs.py b/cylc/flow/job_runner_handlers/pbs.py index b34d288ad94..37545c5b967 100644 --- a/cylc/flow/job_runner_handlers/pbs.py +++ b/cylc/flow/job_runner_handlers/pbs.py @@ -13,7 +13,7 @@ # # You should have received a copy of the GNU General Public License # along with this program. If not, see . -"""Submits task job scripts to PBS (or Torque) by the ``qsub`` command. +"""Submits job scripts to PBS (or Torque) by the ``qsub`` command. .. cylc-scope:: flow.cylc[runtime][] @@ -38,7 +38,7 @@ -q = foo -l nodes = 1 -These are written to the top of the task job script like this: +These are written to the top of the job script like this: .. code-block:: bash diff --git a/cylc/flow/job_runner_handlers/sge.py b/cylc/flow/job_runner_handlers/sge.py index 3e25fef84c9..33f7d5a26d7 100644 --- a/cylc/flow/job_runner_handlers/sge.py +++ b/cylc/flow/job_runner_handlers/sge.py @@ -13,7 +13,7 @@ # # You should have received a copy of the GNU General Public License # along with this program. If not, see . -"""Submits task job scripts to Sun/Oracle Grid Engine with ``qsub``. +"""Submits job scripts to Sun/Oracle Grid Engine with ``qsub``. .. cylc-scope:: flow.cylc[runtime][] @@ -39,7 +39,7 @@ -l h_data = 1024M -l h_rt = 24:00:00 -These are written to the top of the task job script like this: +These are written to the top of the job script like this: .. code-block:: bash diff --git a/cylc/flow/job_runner_handlers/slurm.py b/cylc/flow/job_runner_handlers/slurm.py index 4f848c6ddc0..21c50b0a357 100644 --- a/cylc/flow/job_runner_handlers/slurm.py +++ b/cylc/flow/job_runner_handlers/slurm.py @@ -13,7 +13,7 @@ # # You should have received a copy of the GNU General Public License # along with this program. If not, see . -"""Submits task job scripts to Simple Linux Utility for Resource Management. +"""Submits job scripts to Simple Linux Utility for Resource Management. .. cylc-scope:: flow.cylc[runtime][] @@ -43,7 +43,7 @@ Since not all SLURM commands have a short form, cylc requires the long form directives. -These are written to the top of the task job script like this: +These are written to the top of the job script like this: .. code-block:: bash diff --git a/cylc/flow/job_runner_mgr.py b/cylc/flow/job_runner_mgr.py index 54cba87b3a9..2072f314132 100644 --- a/cylc/flow/job_runner_mgr.py +++ b/cylc/flow/job_runner_mgr.py @@ -282,7 +282,7 @@ def jobs_kill(self, job_log_root, job_log_dirs): """Kill multiple jobs. job_log_root -- The log/job/ sub-directory of the workflow. - job_log_dirs -- A list containing point/name/submit_num for task jobs. + job_log_dirs -- A list containing point/name/submit_num for jobs. """ # Note: The more efficient way to do this is to group the jobs by their @@ -311,7 +311,7 @@ def jobs_poll(self, job_log_root, job_log_dirs): """Poll multiple jobs. job_log_root -- The log/job/ sub-directory of the workflow. - job_log_dirs -- A list containing point/name/submit_num for task jobs. + job_log_dirs -- A list containing point/name/submit_num for jobs. """ if "$" in job_log_root: @@ -368,7 +368,7 @@ def jobs_submit(self, job_log_root, job_log_dirs, remote_mode=False, """Submit multiple jobs. job_log_root -- The log/job/ sub-directory of the workflow. - job_log_dirs -- A list containing point/name/submit_num for task jobs. + job_log_dirs -- A list containing point/name/submit_num for jobs. remote_mode -- am I running on the remote job host? utc_mode -- is the workflow running in UTC mode? diff --git a/cylc/flow/network/schema.py b/cylc/flow/network/schema.py index bed66b86680..2e57d0ad4bd 100644 --- a/cylc/flow/network/schema.py +++ b/cylc/flow/network/schema.py @@ -647,7 +647,7 @@ class Meta: resolver=get_nodes_by_ids) jobs = graphene.List( lambda: Job, - description="""Task jobs.""", + description="""Jobs.""", args=JOB_ARGS, strip_null=STRIP_NULL_DEFAULT, delta_store=DELTA_STORE_DEFAULT, @@ -899,7 +899,7 @@ class Meta: prerequisites = graphene.List(Prerequisite) jobs = graphene.List( Job, - description="""Task jobs.""", + description="""Jobs.""", args=JOB_ARGS, strip_null=STRIP_NULL_DEFAULT, delta_store=DELTA_STORE_DEFAULT, @@ -1518,7 +1518,7 @@ class Meta: description = sstrip(''' Pause a workflow. - This prevents submission of any task jobs. + This suspends submission of tasks. ''') resolver = partial(mutator, command='pause') @@ -1531,15 +1531,15 @@ class Arguments: class Message(Mutation): class Meta: description = sstrip(''' - Record task job messages. + Record task messages. - Send task job messages to: + Send messages to: - The job stdout/stderr. - The job status file, if there is one. - The scheduler, if communication is possible. - Task jobs use this to record and report status such - as success and failure. Applications run by task jobs can use + Jobs use this to record and report status such + as success and failure. Applications run by jobs can use this command to report messages and to report registered task outputs. ''') @@ -1795,7 +1795,7 @@ class Meta: class Poll(Mutation, TaskMutation): class Meta: description = sstrip(''' - Poll (query) task jobs to verify and update their statuses. + Poll (query) jobs to verify and update their statuses. This checks the job status file and queries the job runner on the job platform. @@ -1992,7 +1992,7 @@ class Delta(Interface): ) jobs = graphene.List( Job, - description="""Task jobs.""", + description="""Jobs.""", args=JOB_ARGS, strip_null=Boolean(), delta_store=Boolean(default_value=True), @@ -2067,7 +2067,7 @@ class Meta: ) jobs = graphene.List( Job, - description="""Task jobs.""", + description="""Jobs.""", args=JOB_ARGS, strip_null=Boolean(), delta_store=Boolean(default_value=True), diff --git a/cylc/flow/scripts/cat_log.py b/cylc/flow/scripts/cat_log.py index 2767bacde40..c87af4f2cb8 100755 --- a/cylc/flow/scripts/cat_log.py +++ b/cylc/flow/scripts/cat_log.py @@ -20,7 +20,7 @@ View Cylc workflow and job log files. -Print, tail-follow, print path, or list directory, of local or remote task job +Print, tail-follow, print path, or list directory, of local or remote job and scheduler logs. Job runner view commands (e.g. 'qcat') are used if defined in global config and the job is running. @@ -35,7 +35,7 @@ running, the local (retrieved) log will be accessed unless '-o/--force-remote' is used. -The correct cycle point format of the workflow must be used for task job logs, +The correct cycle point format of the workflow must be used for job logs, but can be discovered with '--mode=d' (print-dir). Examples: diff --git a/cylc/flow/scripts/jobs_submit.py b/cylc/flow/scripts/jobs_submit.py index 61861bae26f..495e9d5eca6 100755 --- a/cylc/flow/scripts/jobs_submit.py +++ b/cylc/flow/scripts/jobs_submit.py @@ -18,7 +18,7 @@ (This command is for internal use.) -Submit task jobs to relevant job runners. +Submit jobs to relevant job runners. On a remote job host, this command reads the job files from STDIN. """ diff --git a/cylc/flow/scripts/message.py b/cylc/flow/scripts/message.py index 5f0b4fb4c16..c7d3606f199 100755 --- a/cylc/flow/scripts/message.py +++ b/cylc/flow/scripts/message.py @@ -17,15 +17,15 @@ # along with this program. If not, see . r"""cylc message [OPTIONS] -- ARGS -Record task job messages. +Record task messages. -Send task job messages to: +Send messages to: - The job stdout/stderr. - The job status file, if there is one. - The scheduler, if communication is possible. -Task jobs use this command to record and report status such as success and -failure. Applications run by task jobs can use this command to report messages +Jobs use this command to record and report status such as success and +failure. Applications run by jobs can use this command to report messages and to report registered task outputs. Messages can be specified as arguments. A '-' indicates that the command should @@ -50,7 +50,7 @@ > WARNING:Hey! >__STDIN__ -Note "${CYLC_WORKFLOW_ID}" and "${CYLC_TASK_JOB}" are available in task job +Note "${CYLC_WORKFLOW_ID}" and "${CYLC_TASK_JOB}" are available in job environments - you do not need to write their actual values in task scripting. Each message can be prefixed with a severity level using the syntax @@ -66,7 +66,7 @@ For backward compatibility, if number of arguments is less than or equal to 2, the command assumes the classic interface, where all arguments are messages. -Otherwise, the first 2 arguments are assumed to be workflow name and task job +Otherwise, the first 2 arguments are assumed to be workflow name and job identifier. """ @@ -98,7 +98,7 @@ def get_option_parser() -> COP: # TODO COP.optional(WORKFLOW_ID_ARG_DOC), COP.optional( - ('TASK-JOB', 'Task job identifier CYCLE/TASK_NAME/SUBMIT_NUM') + ('JOB', 'Job ID - CYCLE/TASK_NAME/SUBMIT_NUM') ), COP.optional( ('[SEVERITY:]MESSAGE ...', 'Messages') diff --git a/cylc/flow/scripts/pause.py b/cylc/flow/scripts/pause.py index ddfc1ebd669..a597a3fe941 100644 --- a/cylc/flow/scripts/pause.py +++ b/cylc/flow/scripts/pause.py @@ -20,7 +20,7 @@ Pause a workflow. -This prevents submission of any task jobs. +This suspends submission of tasks. Examples: # pause my_flow diff --git a/cylc/flow/scripts/poll.py b/cylc/flow/scripts/poll.py index 6a11488321b..2e3508918f5 100755 --- a/cylc/flow/scripts/poll.py +++ b/cylc/flow/scripts/poll.py @@ -18,7 +18,7 @@ """cylc poll [OPTIONS] ARGS -Poll (query) task jobs to verify and update their statuses. +Poll (query) jobs to verify and update their statuses. This checks the job status file and queries the job runner on the job platform. diff --git a/cylc/flow/task_events_mgr.py b/cylc/flow/task_events_mgr.py index 4ac9b8f1595..f928c725756 100644 --- a/cylc/flow/task_events_mgr.py +++ b/cylc/flow/task_events_mgr.py @@ -17,7 +17,7 @@ This module provides logic to: * Manage task messages (internal, polled or received). -* Set up retries on task job failures (submission or execution). +* Set up retries on job failures (submission or execution). * Generate task event handlers. * Retrieval of log files for completed remote jobs. * Email notification. diff --git a/cylc/flow/task_job_logs.py b/cylc/flow/task_job_logs.py index 4804907fdbd..2b88c338ee9 100644 --- a/cylc/flow/task_job_logs.py +++ b/cylc/flow/task_job_logs.py @@ -13,14 +13,14 @@ # # You should have received a copy of the GNU General Public License # along with this program. If not, see . -"""Define task job log filenames and option names.""" +"""Define job log filenames and option names.""" import os from cylc.flow.id import Tokens from cylc.flow.pathutil import get_workflow_run_job_dir -# Task job log filenames. +# job log filenames. JOB_LOG_JOB = "job" JOB_LOG_OUT = "job.out" JOB_LOG_ERR = "job.err" diff --git a/cylc/flow/task_job_mgr.py b/cylc/flow/task_job_mgr.py index f4b2d1b6e73..f5b727ebb7b 100644 --- a/cylc/flow/task_job_mgr.py +++ b/cylc/flow/task_job_mgr.py @@ -13,15 +13,15 @@ # # You should have received a copy of the GNU General Public License # along with this program. If not, see . -"""Manage task jobs. +"""Manage jobs. This module provides logic to: * Set up the directory structure on remote job hosts. * Copy workflow service files to remote job hosts for communication clients. * Clean up of service files on workflow shutdown. -* Prepare task job files. -* Prepare task jobs submission, and manage the callbacks. -* Prepare task jobs poll/kill, and manage the callbacks. +* Prepare job files. +* Prepare jobs submission, and manage the callbacks. +* Prepare jobs poll/kill, and manage the callbacks. """ from contextlib import suppress @@ -121,9 +121,9 @@ class TaskJobManager: """Manage task job submit, poll and kill. This class provides logic to: - * Submit task jobs. - * Poll task jobs. - * Kill task jobs. + * Submit jobs. + * Poll jobs. + * Kill jobs. * Set up the directory structure on job hosts. * Install workflow communicate client files on job hosts. * Remove workflow contact files on job hosts. diff --git a/cylc/flow/task_message.py b/cylc/flow/task_message.py index bd85ccdbd36..d38ce7c8906 100644 --- a/cylc/flow/task_message.py +++ b/cylc/flow/task_message.py @@ -13,9 +13,9 @@ # # You should have received a copy of the GNU General Public License # along with this program. If not, see . -"""Allow a task job to record its messages. +"""Allow a task to record its messages. -Send task job messages to: +Send messages to: - The stdout/stderr. - The job status file, if there is one. - The scheduler, if communication is possible. diff --git a/cylc/flow/task_pool.py b/cylc/flow/task_pool.py index 2b193334b4e..70a3a124a2f 100644 --- a/cylc/flow/task_pool.py +++ b/cylc/flow/task_pool.py @@ -982,7 +982,7 @@ def warn_stop_orphans(self): orphans.append(itask) if orphans_kill_failed: LOG.warning( - "Orphaned task jobs (kill failed):\n" + "Orphaned tasks (kill failed):\n" + "\n".join( f"* {itask.identity} ({itask.state.status})" for itask in orphans_kill_failed @@ -990,7 +990,7 @@ def warn_stop_orphans(self): ) if orphans: LOG.warning( - "Orphaned task jobs:\n" + "Orphaned tasks:\n" + "\n".join( f"* {itask.identity} ({itask.state.status})" for itask in orphans diff --git a/cylc/flow/task_remote_mgr.py b/cylc/flow/task_remote_mgr.py index b728e8d85f4..8d16ace64c0 100644 --- a/cylc/flow/task_remote_mgr.py +++ b/cylc/flow/task_remote_mgr.py @@ -89,7 +89,7 @@ class RemoteTidyQueueTuple(NamedTuple): class TaskRemoteMgr: - """Manage task job remote initialisation, tidy, selection.""" + """Manage task remote initialisation, tidy, selection.""" def __init__(self, workflow, proc_pool, bad_hosts): self.workflow = workflow diff --git a/tests/functional/restart/41-auto-restart-local-jobs.t b/tests/functional/restart/41-auto-restart-local-jobs.t index a1d86d3ef95..0ee771a982d 100644 --- a/tests/functional/restart/41-auto-restart-local-jobs.t +++ b/tests/functional/restart/41-auto-restart-local-jobs.t @@ -109,7 +109,7 @@ log_scan "${TEST_NAME}-stop-log-scan" "${LOG_FILE}" 40 1 \ 'The Cylc workflow host will soon become un-available' \ 'This workflow will be shutdown as the workflow host is unable to continue' \ 'Workflow shutting down - REQUEST(NOW)' \ - 'Orphaned task jobs:' \ + 'Orphaned tasks:' \ '* 1/bar (running)' cylc stop "${WORKFLOW_NAME}" --now --now 2>/dev/null || true diff --git a/tests/functional/shutdown/09-now2.t b/tests/functional/shutdown/09-now2.t index e0aaeb73e77..312aaf63c4d 100755 --- a/tests/functional/shutdown/09-now2.t +++ b/tests/functional/shutdown/09-now2.t @@ -25,7 +25,7 @@ run_ok "${TEST_NAME_BASE}-validate" cylc validate "${WORKFLOW_NAME}" workflow_run_ok "${TEST_NAME_BASE}-run" cylc play --no-detach "${WORKFLOW_NAME}" LOGD="$RUN_DIR/${WORKFLOW_NAME}/log" grep_ok 'INFO - Workflow shutting down - REQUEST(NOW-NOW)' "${LOGD}/scheduler/log" -grep_ok 'WARNING - Orphaned task jobs' "${LOGD}/scheduler/log" +grep_ok 'WARNING - Orphaned tasks' "${LOGD}/scheduler/log" grep_ok '\* 1/t1 (running)' "${LOGD}/scheduler/log" JLOGD="${LOGD}/job/1/t1/01" # Check that 1/t1 event handler runs diff --git a/tests/functional/shutdown/22-stop-now.t b/tests/functional/shutdown/22-stop-now.t index 08cb9c9c3ba..ee8058f0359 100644 --- a/tests/functional/shutdown/22-stop-now.t +++ b/tests/functional/shutdown/22-stop-now.t @@ -43,7 +43,7 @@ run_ok "${TEST_NAME_BASE}" cylc play "${WORKFLOW_NAME}" --no-detach --debug WORKFLOW_LOG="${WORKFLOW_RUN_DIR}/log/scheduler/log" log_scan "${TEST_NAME_BASE}-orphaned" "${WORKFLOW_LOG}" 1 1 \ - 'Orphaned task jobs.*' \ + 'Orphaned tasks.*' \ '1/.*foo' purge diff --git a/tests/integration/test_scheduler.py b/tests/integration/test_scheduler.py index c4578ef6a70..629c62d7f57 100644 --- a/tests/integration/test_scheduler.py +++ b/tests/integration/test_scheduler.py @@ -242,7 +242,7 @@ async def test_no_poll_waiting_tasks( task.state.status = TASK_STATUS_RUNNING # For good measure, check the faked running task is reported at shutdown. - assert "Orphaned task jobs:\n* 1/one (running)" in log.messages + assert "Orphaned tasks:\n* 1/one (running)" in log.messages @pytest.mark.parametrize('reload', [False, True]) diff --git a/tests/unit/test_job_file.py b/tests/unit/test_job_file.py index 5f157af654b..b779d3803e3 100644 --- a/tests/unit/test_job_file.py +++ b/tests/unit/test_job_file.py @@ -116,7 +116,7 @@ def test_write(fixture_get_platform): """Test the header is correctly written""" - expected = ('#!/bin/bash -l\n#\n# ++++ THIS IS A CYLC TASK JOB SCRIPT ' + expected = ('#!/bin/bash -l\n#\n# ++++ THIS IS A CYLC JOB SCRIPT ' '++++\n# Workflow: farm_noises\n# Task: 1/baa\n# Job ' 'log directory: 1/baa/01\n# Job runner: ' 'background\n# Job runner command template: woof\n#' From e9adbe981cfe5562c036872d87083ecbea6eb6e5 Mon Sep 17 00:00:00 2001 From: Oliver Sanders Date: Mon, 12 Sep 2022 13:21:44 +0100 Subject: [PATCH 34/66] Remove HOME Env Variable from get_remote_workflow_run_dir (#5115) (#5128) * Remove HOME env var from rsync commands Co-authored-by: Melanie Hall <37735232+datamel@users.noreply.github.com> --- CHANGES.md | 3 +++ cylc/flow/remote.py | 1 + cylc/flow/subprocpool.py | 2 +- cylc/flow/task_events_mgr.py | 4 +++- .../events/17-task-event-job-logs-retrieve-command.t | 2 +- 5 files changed, 9 insertions(+), 3 deletions(-) diff --git a/CHANGES.md b/CHANGES.md index 22e481d4c68..3b7952228b8 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -48,6 +48,9 @@ Maintenance release. ### Fixes +[#5115](https://github.com/cylc/cylc-flow/pull/5115) - Updates rsync commands +to make them compatible with latest rsync releases. + [#5119](https://github.com/cylc/cylc-flow/pull/5119) - Fix formatting of deprecation warnings at validation. diff --git a/cylc/flow/remote.py b/cylc/flow/remote.py index c8541d8260e..ccbb5558000 100644 --- a/cylc/flow/remote.py +++ b/cylc/flow/remote.py @@ -193,6 +193,7 @@ def construct_rsync_over_ssh_cmd( ``rsync_cmd[0] == 'rsync'``. Please check that changes to this funtion do not break ``rsync_255_fail``. """ + dst_path = dst_path.replace('$HOME/', '') dst_host = get_host_from_platform(platform, bad_hosts=bad_hosts) ssh_cmd = platform['ssh command'] command = platform['rsync command'] diff --git a/cylc/flow/subprocpool.py b/cylc/flow/subprocpool.py index ec14fc6e2ea..05648631fb3 100644 --- a/cylc/flow/subprocpool.py +++ b/cylc/flow/subprocpool.py @@ -549,7 +549,7 @@ def rsync_255_fail(ctx, platform=None) -> bool: """Tests context for rsync failing to communicate with a host. If there has been a failure caused by rsync being unable to connect - try a test of ssh connectivity. Necessary becuase loss of connectivity + try a test of ssh connectivity. Necessary because loss of connectivity may cause different rsync failures depending on version, and some of the failures may be caused by other problems. """ diff --git a/cylc/flow/task_events_mgr.py b/cylc/flow/task_events_mgr.py index f928c725756..b0386ccc292 100644 --- a/cylc/flow/task_events_mgr.py +++ b/cylc/flow/task_events_mgr.py @@ -957,7 +957,9 @@ def _process_job_logs_retrieval(self, schd, ctx, id_keys): # Remote source cmd.append("%s:%s/" % ( host, - get_remote_workflow_run_job_dir(schd.workflow))) + get_remote_workflow_run_job_dir( + schd.workflow).replace('$HOME/', '')) + ) # Local target cmd.append(get_workflow_run_job_dir(schd.workflow) + "/") self.proc_pool.put_command( diff --git a/tests/functional/events/17-task-event-job-logs-retrieve-command.t b/tests/functional/events/17-task-event-job-logs-retrieve-command.t index 221a27fcaa3..76245489735 100755 --- a/tests/functional/events/17-task-event-job-logs-retrieve-command.t +++ b/tests/functional/events/17-task-event-job-logs-retrieve-command.t @@ -52,7 +52,7 @@ sed 's/^.* -v //' "${WORKFLOW_LOG_D}/scheduler/my-rsync.log" >'my-rsync.log.edit OPT_HEAD='--include=/1 --include=/1/t1' OPT_TAIL='--exclude=/**' -ARGS="${CYLC_TEST_HOST}:\$HOME/cylc-run/${WORKFLOW_NAME}/log/job/ ${WORKFLOW_LOG_D}/job/" +ARGS="${CYLC_TEST_HOST}:cylc-run/${WORKFLOW_NAME}/log/job/ ${WORKFLOW_LOG_D}/job/" cmp_ok 'my-rsync.log.edited' <<__LOG__ ${OPT_HEAD} --include=/1/t1/01 --include=/1/t1/01/** ${OPT_TAIL} ${ARGS} ${OPT_HEAD} --include=/1/t1/02 --include=/1/t1/02/** ${OPT_TAIL} ${ARGS} From 83f258c85ef6351de3e84cc0e5cfb6c5aa85d318 Mon Sep 17 00:00:00 2001 From: Oliver Sanders Date: Tue, 13 Sep 2022 11:01:11 +0100 Subject: [PATCH 35/66] changelog: strip pre 8.0.0 releases (#5130) --- CHANGES.md | 1198 ++-------------------------------------------------- README.md | 26 ++ 2 files changed, 51 insertions(+), 1173 deletions(-) diff --git a/CHANGES.md b/CHANGES.md index 3b7952228b8..c6587afaae0 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -1,27 +1,8 @@ -# Selected Cylc Changes +# Changelog -Internal changes that do not directly affect users may not be listed here. For -all changes see the [closed -milestones](https://github.com/cylc/cylc-flow/milestones?state=closed) for each -release. - -## Major Changes in Cylc 8 - -* Python 2 -> 3. -* Internal communications converted from HTTPS to ZMQ (TCP). -* PyGTK GUIs replaced by: - * Terminal user interface (TUI) included in cylc-flow. - * Web user interface provided by the cylc-uiserver package. -* A new scheduling algorithm with support for branched workflows. -* Command line changes: - * `cylc run` -> `cylc play` - * `cylc restart` -> `cylc play` - * `rose suite-run` -> `cylc install; cylc play ` -* The core package containing Cylc scheduler program has been renamed cylc-flow. -* Cylc review has been removed, the Cylc 7 version remains Cylc 8 compatible. -* [New documentation](https://cylc.github.io/cylc-doc/stable). - -See the [migration guide](https://cylc.github.io/cylc-doc/stable/html/7-to-8/index.html) for a full list of changes. +List of notable changes, for a complete list of changes see the +[closed milestones](https://github.com/cylc/cylc-flow/milestones?state=closed) +for each release. + ------------------------------------------------------------------------------- ## __cylc-8.1.0 (Upcoming)__ @@ -29,9 +30,22 @@ Maintenance release. ### Fixes +[#5023](https://github.com/cylc/cylc-flow/pull/5023) - tasks force-triggered +after a shutdown was ordered should submit to run immediately on restart. + +[#5137](https://github.com/cylc/cylc-flow/pull/5137) - +Install the `ana/` directory to remote platforms by default. + +[#5146](https://github.com/cylc/cylc-flow/pull/5146) - no-flow tasks should not +retrigger incomplete children. + [#5104](https://github.com/cylc/cylc-flow/pull/5104) - Fix retriggering of failed tasks after a reload. +[#5139](https://github.com/cylc/cylc-flow/pull/5139) - Fix bug where +`cylc install` could hang if there was a large uncommitted diff in the +source dir (for git/svn repos). + [#5131](https://github.com/cylc/cylc-flow/pull/5131) - Infer workflow run number for `workflow_state` xtrigger. diff --git a/cylc/flow/cfgspec/workflow.py b/cylc/flow/cfgspec/workflow.py index f8948588d1e..10d199d6832 100644 --- a/cylc/flow/cfgspec/workflow.py +++ b/cylc/flow/cfgspec/workflow.py @@ -222,10 +222,17 @@ def get_script_common_text(this: str, example: Optional[str] = None): The following directories already get installed by default: - * ``app/`` - * ``bin/`` - * ``etc/`` - * ``lib/`` + ``ana/`` + Rose ana analysis modules + ``app/`` + Rose applications + ``bin/`` + Cylc bin directory (added to ``PATH``) + ``etc/`` + Miscellaneous resources + ``lib/`` + Cylc lib directory (``lib/python`` added to ``PYTHONPATH`` + for workflow config) These should be located in the top level of your Cylc workflow, i.e. the directory that contains your ``flow.cylc`` file. diff --git a/cylc/flow/install_plugins/log_vc_info.py b/cylc/flow/install_plugins/log_vc_info.py index 9be634c5b0f..1aec3eecd5d 100644 --- a/cylc/flow/install_plugins/log_vc_info.py +++ b/cylc/flow/install_plugins/log_vc_info.py @@ -62,7 +62,9 @@ import json from pathlib import Path from subprocess import Popen, DEVNULL, PIPE -from typing import Any, Dict, Iterable, List, Optional, TYPE_CHECKING, Union +from typing import ( + Any, Dict, Iterable, List, Optional, TYPE_CHECKING, TextIO, Union, overload +) from cylc.flow import LOG from cylc.flow.exceptions import CylcError @@ -80,8 +82,6 @@ GIT: ['describe', '--always', '--dirty'] } -# git ['show', '--quiet', '--format=short'], - STATUS_COMMANDS: Dict[str, List[str]] = { SVN: ['status', '--non-interactive'], GIT: ['status', '--short'] @@ -189,13 +189,40 @@ def get_vc_info(path: Union[Path, str]) -> Optional[Dict[str, Any]]: return None -def _run_cmd(vcs: str, args: Iterable[str], cwd: Union[Path, str]) -> str: - """Run a VCS command, return stdout. +@overload +def _run_cmd( + vcs: str, args: Iterable[str], cwd: Union[Path, str], stdout: int = PIPE +) -> str: + ... + + +@overload +def _run_cmd( + vcs: str, args: Iterable[str], cwd: Union[Path, str], stdout: TextIO +) -> None: + ... + + +def _run_cmd( + vcs: str, + args: Iterable[str], + cwd: Union[Path, str], + stdout: Union[TextIO, int] = PIPE +) -> Optional[str]: + """Run a VCS command. Args: vcs: The version control system. args: The args to pass to the version control command. cwd: Directory to run the command in. + stdout: Where to redirect output (either PIPE or a + text stream/file object). Note: only use PIPE for + commands that will not generate a large output, otherwise + the pipe might get blocked. + + Returns: + Stdout output if stdout=PIPE, else None as the output has been + written directly to the specified file. Raises: VCSNotInstalledError: The VCS is not found. @@ -208,7 +235,7 @@ def _run_cmd(vcs: str, args: Iterable[str], cwd: Union[Path, str]) -> str: cmd, cwd=cwd, stdin=DEVNULL, - stdout=PIPE, + stdout=stdout, stderr=PIPE, text=True, ) @@ -275,41 +302,40 @@ def _parse_svn_info(info_text: str) -> Dict[str, Any]: return ret -def get_diff(vcs: str, path: Union[Path, str]) -> Optional[str]: - """Return the diff of uncommitted changes for a repository. +def write_diff( + vcs: str, repo_path: Union[Path, str], run_dir: Union[Path, str] +) -> Path: + """Get and write the diff of uncommitted changes for a repository to the + workflow's vcs log dir. Args: vcs: The version control system. - path: The path to the repo. - """ - args_ = DIFF_COMMANDS[vcs] - if Path(path).is_absolute(): - args_.append(str(path)) - else: - args_.append(str(Path().cwd() / path)) - - try: - diff = _run_cmd(vcs, args_, cwd=path) - except (VCSNotInstalledError, VCSMissingBaseError): - return None - header = ( - "# Auto-generated diff of uncommitted changes in the Cylc " - "workflow repository:\n" - f"# {path}") - return f"{header}\n{diff}" - - -def write_diff(diff: str, run_dir: Union[Path, str]) -> None: - """Write a diff to the workflow's vcs log dir. - - Args: - diff: The diff. + repo_path: The path to the repo. run_dir: The workflow run directory. + + Returns the path to diff file. """ + args = DIFF_COMMANDS[vcs] + args.append( + str(repo_path) if Path(repo_path).is_absolute() else + str(Path().cwd() / repo_path) + ) + diff_file = Path(run_dir, LOG_VERSION_DIR, DIFF_FILENAME) diff_file.parent.mkdir(exist_ok=True) - with open(diff_file, 'w') as f: - f.write(diff) + + with open(diff_file, 'a') as f: + f.write( + "# Auto-generated diff of uncommitted changes in the Cylc " + "workflow repository:\n" + f"# {repo_path}\n" + ) + f.flush() + try: + _run_cmd(vcs, args, repo_path, stdout=f) + except VCSMissingBaseError as exc: + f.write(f"# No diff - {exc}") + return diff_file # Entry point: @@ -331,8 +357,6 @@ def main( if vc_info is None: return False vcs = vc_info['version control system'] - diff = get_diff(vcs, srcdir) write_vc_info(vc_info, rundir) - if diff is not None: - write_diff(diff, rundir) + write_diff(vcs, srcdir, rundir) return True diff --git a/cylc/flow/remote.py b/cylc/flow/remote.py index ccbb5558000..9bc0fe36d53 100644 --- a/cylc/flow/remote.py +++ b/cylc/flow/remote.py @@ -172,6 +172,15 @@ def get_includes_to_rsync(rsync_includes=None): '--no-t' ] +DEFAULT_INCLUDES = [ + '/ana/***', # Rose ana analysis modules + '/app/***', # Rose applications + '/bin/***', # Cylc bin directory (added to PATH) + '/etc/***', # Miscellaneous resources + '/lib/***', # Cylc lib directory (lib/python added to PYTHONPATH for + # workflow config) +] + def construct_rsync_over_ssh_cmd( src_path: str, dst_path: str, platform: Dict[str, Any], @@ -209,12 +218,7 @@ def construct_rsync_over_ssh_cmd( rsync_cmd.extend(rsync_options) for exclude in ['log', 'share', 'work']: rsync_cmd.append(f"--exclude={exclude}") - default_includes = [ - '/app/***', - '/bin/***', - '/etc/***', - '/lib/***'] - for include in default_includes: + for include in DEFAULT_INCLUDES: rsync_cmd.append(f"--include={include}") for include in get_includes_to_rsync(rsync_includes): rsync_cmd.append(f"--include={include}") diff --git a/cylc/flow/rundb.py b/cylc/flow/rundb.py index 4cb3c630638..42a4cca3e9a 100644 --- a/cylc/flow/rundb.py +++ b/cylc/flow/rundb.py @@ -297,6 +297,7 @@ class CylcWorkflowDAO: ["submit_num", {"datatype": "INTEGER"}], ["status"], ["flow_wait", {"datatype": "INTEGER"}], + ["is_manual_submit", {"datatype": "INTEGER"}], ], TABLE_TASK_TIMEOUT_TIMERS: [ ["cycle", {"is_primary_key": True}], @@ -802,14 +803,15 @@ def select_task_pool_for_restart(self, callback): """Select from task_pool+task_states+task_jobs for restart. Invoke callback(row_idx, row) on each row, where each row contains: - [cycle, name, is_late, status, is_held, submit_num, - try_num, platform_name, time_submit, time_run, timeout, outputs] + the fields in the SELECT statement below. """ form_stmt = r""" SELECT %(task_pool)s.cycle, %(task_pool)s.name, %(task_pool)s.flow_nums, + %(task_states)s.flow_wait, + %(task_states)s.is_manual_submit, %(task_late_flags)s.value, %(task_pool)s.status, %(task_pool)s.is_held, diff --git a/cylc/flow/task_job_mgr.py b/cylc/flow/task_job_mgr.py index 28848badd0d..8b3acb9ce06 100644 --- a/cylc/flow/task_job_mgr.py +++ b/cylc/flow/task_job_mgr.py @@ -238,7 +238,6 @@ def prep_submit_task_jobs(self, workflow, itasks, check_syntax=True): itask.submit_num += 1 itask.state_reset(TASK_STATUS_PREPARING) self.data_store_mgr.delta_task_state(itask) - self.workflow_db_mgr.put_update_task_state(itask) prep_task = self._prep_submit_task_job( workflow, itask, check_syntax=check_syntax) if prep_task: diff --git a/cylc/flow/task_pool.py b/cylc/flow/task_pool.py index 2a30b978fd6..2582f0a508f 100644 --- a/cylc/flow/task_pool.py +++ b/cylc/flow/task_pool.py @@ -174,13 +174,32 @@ def load_from_point(self): point = tdef.first_point(self.config.start_point) self.spawn_to_rh_limit(tdef, point, {flow_num}) - def add_to_pool(self, itask, is_new: bool = True) -> None: + def db_add_new_flow_rows(self, itask: TaskProxy) -> None: + """Add new rows to DB task tables that record flow_nums. + + Call when a new task is spawned or a flow merge occurs. + """ + # Add row to task_states table. + now = get_current_time_string() + self.workflow_db_mgr.put_insert_task_states( + itask, + { + "time_created": now, + "time_updated": now, + "status": itask.state.status, + "flow_nums": serialise(itask.flow_nums), + "flow_wait": itask.flow_wait, + "is_manual_submit": itask.is_manual_submit + } + ) + # Add row to task_outputs table: + self.workflow_db_mgr.put_insert_task_outputs(itask) + + def add_to_pool(self, itask) -> None: """Add a task to the hidden (if not satisfied) or main task pool. If the task already exists in the hidden pool and is satisfied, move it to the main pool. - - (is_new is False inidcates load from DB at restart). """ if itask.is_task_prereqs_not_done() and not itask.is_manual_submit: # Add to hidden pool if not satisfied. @@ -206,21 +225,6 @@ def add_to_pool(self, itask, is_new: bool = True) -> None: self.create_data_store_elements(itask) - if is_new: - # Add row to "task_states" table. - now = get_current_time_string() - self.workflow_db_mgr.put_insert_task_states( - itask, - { - "time_created": now, - "time_updated": now, - "status": itask.state.status, - "flow_nums": serialise(itask.flow_nums) - } - ) - # Add row to "task_outputs" table: - self.workflow_db_mgr.put_insert_task_outputs(itask) - if itask.tdef.max_future_prereq_offset is not None: # (Must do this once added to the pool). self.set_max_future_offset() @@ -417,9 +421,9 @@ def load_db_task_pool_for_restart(self, row_idx, row): if row_idx == 0: LOG.info("LOADING task proxies") # Create a task proxy corresponding to this DB entry. - (cycle, name, flow_nums, is_late, status, is_held, submit_num, _, - platform_name, time_submit, time_run, timeout, outputs_str) = row - + (cycle, name, flow_nums, flow_wait, is_manual_submit, is_late, status, + is_held, submit_num, _, platform_name, time_submit, time_run, timeout, + outputs_str) = row try: itask = TaskProxy( self.config.get_taskdef(name), @@ -428,7 +432,9 @@ def load_db_task_pool_for_restart(self, row_idx, row): status=status, is_held=is_held, submit_num=submit_num, - is_late=bool(is_late) + is_late=bool(is_late), + flow_wait=bool(flow_wait), + is_manual_submit=bool(is_manual_submit) ) except WorkflowConfigError: LOG.exception( @@ -492,7 +498,7 @@ def load_db_task_pool_for_restart(self, row_idx, row): if itask.state_reset(status, is_runahead=True): self.data_store_mgr.delta_task_runahead(itask) - self.add_to_pool(itask, is_new=False) + self.add_to_pool(itask) # All tasks load as runahead-limited, but finished and manually # triggered tasks (incl. --start-task's) can be released now. @@ -629,8 +635,9 @@ def _get_spawned_or_merged_task( def spawn_to_rh_limit(self, tdef, point, flow_nums) -> None: """Spawn parentless task instances from point to runahead limit.""" - if not flow_nums: - # force-triggered no-flow task. + if not flow_nums or point is None: + # Force-triggered no-flow task. + # Or called with an invalid next_point. return if self.runahead_limit_point is None: self.compute_runahead() @@ -1206,14 +1213,6 @@ def spawn_on_output(self, itask, output, forced=False): if c_task is not None and c_task != itask: # (Avoid self-suicide: A => !A) self.merge_flows(c_task, itask.flow_nums) - self.workflow_db_mgr.put_insert_task_states( - c_task, - { - "status": c_task.state.status, - "flow_nums": serialise(c_task.flow_nums) - } - ) - # self.workflow_db_mgr.process_queued_ops() elif ( c_task is None and (itask.flow_nums or forced) @@ -1488,6 +1487,7 @@ def spawn_task( return None LOG.info(f"[{itask}] spawned") + self.db_add_new_flow_rows(itask) return itask def force_spawn_children( @@ -1593,7 +1593,6 @@ def force_trigger_tasks( ) if itask is None: continue - self.add_to_pool(itask, is_new=True) itasks.append(itask) # Trigger matched tasks if not already active. @@ -1621,7 +1620,6 @@ def force_trigger_tasks( # De-queue it to run now. self.task_queue_mgr.force_release_task(itask) - self.workflow_db_mgr.put_update_task_state(itask) return len(unmatched) def sim_time_check(self, message_queue): @@ -1925,31 +1923,35 @@ def merge_flows(self, itask: TaskProxy, flow_nums: 'FlowNums') -> None: This also performs required spawning / state changing for edge cases. """ - if flow_nums == itask.flow_nums: - # Don't do anything if trying to spawn the same task in the same - # flow. This arises downstream of an AND trigger (if "A & B => C" + if not flow_nums or (flow_nums == itask.flow_nums): + # Don't do anything if: + # 1. merging from a no-flow task, or + # 2. trying to spawn the same task in the same flow. This arises + # downstream of an AND trigger (if "A & B => C" # and A spawns C first, B will find C is already in the pool), # and via suicide triggers ("A =>!A": A tries to spawn itself). return + merge_with_no_flow = not itask.flow_nums + + itask.merge_flows(flow_nums) + # Merged tasks get a new row in the db task_states table. + self.db_add_new_flow_rows(itask) + if ( itask.state(*TASK_STATUSES_FINAL) and itask.state.outputs.get_incomplete() ): # Re-queue incomplete task to run again in the merged flow. LOG.info(f"[{itask}] incomplete task absorbed by new flow.") - itask.merge_flows(flow_nums) itask.state_reset(TASK_STATUS_WAITING) self.queue_task(itask) self.data_store_mgr.delta_task_state(itask) - elif not itask.flow_nums or itask.flow_wait: + elif merge_with_no_flow or itask.flow_wait: # 2. Retro-spawn on completed outputs and continue as merged flow. LOG.info(f"[{itask}] spawning on pre-merge outputs") - itask.merge_flows(flow_nums) itask.flow_wait = False self.spawn_on_all_outputs(itask, completed_only=True) self.spawn_to_rh_limit( itask.tdef, itask.next_point(), itask.flow_nums) - else: - itask.merge_flows(flow_nums) diff --git a/cylc/flow/workflow_db_mgr.py b/cylc/flow/workflow_db_mgr.py index e8f82a27669..18265d67b79 100644 --- a/cylc/flow/workflow_db_mgr.py +++ b/cylc/flow/workflow_db_mgr.py @@ -431,14 +431,15 @@ def put_xtriggers(self, sat_xtrig): def put_update_task_state(self, itask): """Update task_states table for current state of itask. - For final event-driven update before removing finished tasks. - No need to update task_pool table as finished tasks are immediately - removed from the pool. + NOTE the task_states table is normally updated along with the task pool + table. This method is only needed as a final update for finished tasks, + when they get removed from the task_pool. """ set_args = { "time_updated": itask.state.time_updated, "status": itask.state.status, - "flow_wait": itask.flow_wait + "flow_wait": itask.flow_wait, + "is_manual_submit": itask.is_manual_submit } where_args = { "cycle": str(itask.point), @@ -451,10 +452,15 @@ def put_update_task_state(self, itask): (set_args, where_args)) def put_task_pool(self, pool: 'TaskPool') -> None: - """Update various task tables for current pool, in runtime database. + """Delete task pool table content and recreate from current task pool. - Queue delete (everything) statements to wipe the tables, and queue the - relevant insert statements for the current tasks in the pool. + Also recreate: + - prerequisites table + - timeout timers table + - action timers table + + And update: + - task states table """ self.db_deletes_map[self.TABLE_TASK_POOL].append({}) # Comment this out to retain the trigger-time prereq status of past diff --git a/tests/flakyfunctional/database/00-simple/schema.out b/tests/flakyfunctional/database/00-simple/schema.out index ac62d48639d..814faac2a59 100644 --- a/tests/flakyfunctional/database/00-simple/schema.out +++ b/tests/flakyfunctional/database/00-simple/schema.out @@ -10,7 +10,7 @@ CREATE TABLE task_late_flags(cycle TEXT, name TEXT, value INTEGER, PRIMARY KEY(c CREATE TABLE task_outputs(cycle TEXT, name TEXT, flow_nums TEXT, outputs TEXT, PRIMARY KEY(cycle, name, flow_nums)); CREATE TABLE task_pool(cycle TEXT, name TEXT, flow_nums TEXT, status TEXT, is_held INTEGER, PRIMARY KEY(cycle, name, flow_nums)); CREATE TABLE task_prerequisites(cycle TEXT, name TEXT, flow_nums TEXT, prereq_name TEXT, prereq_cycle TEXT, prereq_output TEXT, satisfied TEXT, PRIMARY KEY(cycle, name, flow_nums, prereq_name, prereq_cycle, prereq_output)); -CREATE TABLE task_states(name TEXT, cycle TEXT, flow_nums TEXT, time_created TEXT, time_updated TEXT, submit_num INTEGER, status TEXT, flow_wait INTEGER, PRIMARY KEY(name, cycle, flow_nums)); +CREATE TABLE task_states(name TEXT, cycle TEXT, flow_nums TEXT, time_created TEXT, time_updated TEXT, submit_num INTEGER, status TEXT, flow_wait INTEGER, is_manual_submit INTEGER, PRIMARY KEY(name, cycle, flow_nums)); CREATE TABLE task_timeout_timers(cycle TEXT, name TEXT, timeout REAL, PRIMARY KEY(cycle, name)); CREATE TABLE tasks_to_hold(name TEXT, cycle TEXT); CREATE TABLE workflow_flows(flow_num INTEGER, start_time TEXT, description TEXT, PRIMARY KEY(flow_num)); diff --git a/tests/functional/flow-triggers/11-wait-merge.t b/tests/functional/flow-triggers/11-wait-merge.t index 25b6443776a..cb3218ae463 100644 --- a/tests/functional/flow-triggers/11-wait-merge.t +++ b/tests/functional/flow-triggers/11-wait-merge.t @@ -34,8 +34,8 @@ cmp_ok "${TEST_NAME}.stdout" <<\__END__ 1|b|[1]|["submitted", "started", "succeeded"] 1|a|[2]|["submitted", "started", "succeeded"] 1|c|[2]|["submitted", "started", "x"] -1|x|[1, 2]|["submitted", "started", "succeeded"] 1|c|[1, 2]|["submitted", "started", "succeeded", "x"] +1|x|[1, 2]|["submitted", "started", "succeeded"] 1|d|[1, 2]|["submitted", "started", "succeeded"] 1|b|[2]|["submitted", "started", "succeeded"] __END__ diff --git a/tests/functional/flow-triggers/13-noflow-nomerge.t b/tests/functional/flow-triggers/13-noflow-nomerge.t new file mode 100644 index 00000000000..c8b4528a2f9 --- /dev/null +++ b/tests/functional/flow-triggers/13-noflow-nomerge.t @@ -0,0 +1,47 @@ +#!/usr/bin/env bash +# THIS FILE IS PART OF THE CYLC WORKFLOW ENGINE. +# Copyright (C) NIWA & British Crown (Met Office) & Contributors. +# +# This program is free software: you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation, either version 3 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program. If not, see . +#------------------------------------------------------------------------------- + +. "$(dirname "$0")/test_header" +set_test_number 7 + +install_workflow "${TEST_NAME_BASE}" "${TEST_NAME_BASE}" +DB="${WORKFLOW_RUN_DIR}/log/db" + +run_ok "${TEST_NAME_BASE}-validate" cylc validate "${WORKFLOW_NAME}" +run_ok "${TEST_NAME_BASE}-run" cylc play "${WORKFLOW_NAME}" +poll_grep_workflow_log "Workflow stalled" + +run_ok "${TEST_NAME_BASE}-trigger" cylc trigger --flow=none "${WORKFLOW_NAME}//1/a" +poll_grep_workflow_log -E "1/a running job:02 flows:none.*=> succeeded" + +cylc stop --now --now --max-polls=5 --interval=2 "$WORKFLOW_NAME" + +TEST_NAME="${TEST_NAME_BASE}-count" +QUERY="SELECT COUNT(*) FROM task_states WHERE name=='a'" +run_ok "${TEST_NAME}" sqlite3 "${DB}" "$QUERY" +cmp_ok "${TEST_NAME}.stdout" <<__END__ +2 +__END__ + +QUERY="SELECT COUNT(*) FROM task_states WHERE name=='b'" +run_ok "${TEST_NAME}" sqlite3 "${DB}" "$QUERY" +cmp_ok "${TEST_NAME}.stdout" <<__END__ +1 +__END__ + +purge diff --git a/tests/functional/flow-triggers/13-noflow-nomerge/flow.cylc b/tests/functional/flow-triggers/13-noflow-nomerge/flow.cylc new file mode 100644 index 00000000000..9b7e2b90a25 --- /dev/null +++ b/tests/functional/flow-triggers/13-noflow-nomerge/flow.cylc @@ -0,0 +1,7 @@ +[scheduling] + [[graph]] + R1 = "a => b" +[runtime] + [[a]] + [[b]] + script = false # die as incomplete diff --git a/tests/functional/job-submission/01-job-nn-localhost/db.sqlite3 b/tests/functional/job-submission/01-job-nn-localhost/db.sqlite3 index 9453b6960e3..15d9e84418f 100644 --- a/tests/functional/job-submission/01-job-nn-localhost/db.sqlite3 +++ b/tests/functional/job-submission/01-job-nn-localhost/db.sqlite3 @@ -18,8 +18,8 @@ CREATE TABLE task_late_flags(cycle TEXT, name TEXT, value INTEGER, PRIMARY KEY(c CREATE TABLE task_outputs(cycle TEXT, name TEXT, flow_nums TEXT, outputs TEXT, PRIMARY KEY(cycle, name, flow_nums)); CREATE TABLE task_pool(cycle TEXT, name TEXT, flow_nums TEXT, status TEXT, is_held INTEGER, PRIMARY KEY(cycle, name, flow_nums)); INSERT INTO task_pool VALUES('1','foo','["1", "2"]','waiting', 0); -CREATE TABLE task_states(name TEXT, cycle TEXT, flow_nums TEXT, time_created TEXT, time_updated TEXT, submit_num INTEGER, status TEXT, flow_wait INTEGER, PRIMARY KEY(name, cycle, flow_nums)); -INSERT INTO task_states VALUES('foo','1','["1", "2"]', '2019-06-14T11:30:16+01:00','2019-06-14T11:40:24+01:00',99,'waiting','0'); +CREATE TABLE task_states(name TEXT, cycle TEXT, flow_nums TEXT, time_created TEXT, time_updated TEXT, submit_num INTEGER, status TEXT, flow_wait INTEGER, is_manual_submit INTEGER, PRIMARY KEY(name, cycle, flow_nums)); +INSERT INTO task_states VALUES('foo','1','["1", "2"]', '2019-06-14T11:30:16+01:00','2019-06-14T11:40:24+01:00',99,'waiting','0', '0'); CREATE TABLE task_prerequisites(cycle TEXT, name TEXT, flow_nums TEXT, prereq_name TEXT, prereq_cycle TEXT, prereq_output TEXT, satisfied TEXT, PRIMARY KEY(cycle, name, flow_nums, prereq_name, prereq_cycle, prereq_output)); CREATE TABLE task_timeout_timers(cycle TEXT, name TEXT, timeout REAL, PRIMARY KEY(cycle, name)); CREATE TABLE xtriggers(signature TEXT, results TEXT, PRIMARY KEY(signature)); diff --git a/tests/functional/remote/01-file-install.t b/tests/functional/remote/01-file-install.t index e673b7c7c9d..9f89c49f79a 100644 --- a/tests/functional/remote/01-file-install.t +++ b/tests/functional/remote/01-file-install.t @@ -23,7 +23,7 @@ set_test_number 6 create_files () { # dump some files into the run dir - for DIR in "bin" "app" "etc" "lib" "dir1" "dir2" + for DIR in "bin" "ana" "app" "etc" "lib" "dir1" "dir2" do mkdir -p "${WORKFLOW_RUN_DIR}/${DIR}" touch "${WORKFLOW_RUN_DIR}/${DIR}/moo" @@ -35,7 +35,7 @@ create_files () { } # Test configured files/directories along with default files/directories -# (app, bin, etc, lib) are correctly installed on the remote platform. +# (ana, app, bin, etc, lib) are correctly installed on the remote platform. TEST_NAME="${TEST_NAME_BASE}-default-paths" init_workflow "${TEST_NAME}" <<__FLOW_CONFIG__ [scheduling] @@ -59,8 +59,9 @@ workflow_run_ok "${TEST_NAME}-run1" cylc play "${WORKFLOW_NAME}" \ # ensure these files get installed on the remote platform SSH="$(cylc config -d -i "[platforms][$CYLC_TEST_PLATFORM]ssh command")" ${SSH} "${CYLC_TEST_HOST}" \ - find "${RUN_DIR_REL}/"{app,bin,etc,lib} -type f | sort > 'find.out' + find "${RUN_DIR_REL}/"{ana,app,bin,etc,lib} -type f | sort > 'find.out' cmp_ok 'find.out' <<__OUT__ +${RUN_DIR_REL}/ana/moo ${RUN_DIR_REL}/app/moo ${RUN_DIR_REL}/bin/moo ${RUN_DIR_REL}/etc/moo @@ -93,8 +94,9 @@ workflow_run_ok "${TEST_NAME}-run2" cylc play "${WORKFLOW_NAME}" \ -s "CYLC_TEST_PLATFORM='${CYLC_TEST_PLATFORM}'" ${SSH} "${CYLC_TEST_HOST}" \ - find "${RUN_DIR_REL}/"{app,bin,dir1,dir2,file1,file2,etc,lib} -type f | sort > 'find.out' + find "${RUN_DIR_REL}/"{ana,app,bin,dir1,dir2,file1,file2,etc,lib} -type f | sort > 'find.out' cmp_ok 'find.out' <<__OUT__ +${RUN_DIR_REL}/ana/moo ${RUN_DIR_REL}/app/moo ${RUN_DIR_REL}/bin/moo ${RUN_DIR_REL}/dir1/moo diff --git a/tests/functional/restart/57-ghost-job/db.sqlite3 b/tests/functional/restart/57-ghost-job/db.sqlite3 index d6837d6bd0c..4230831602f 100644 --- a/tests/functional/restart/57-ghost-job/db.sqlite3 +++ b/tests/functional/restart/57-ghost-job/db.sqlite3 @@ -19,8 +19,8 @@ INSERT INTO task_outputs VALUES('1','foo','[1]','[]'); CREATE TABLE task_pool(cycle TEXT, name TEXT, flow_nums TEXT, status TEXT, is_held INTEGER, PRIMARY KEY(cycle, name, flow_nums)); INSERT INTO task_pool VALUES('1','foo','[1]','preparing',0); CREATE TABLE task_prerequisites(cycle TEXT, name TEXT, flow_nums TEXT, prereq_name TEXT, prereq_cycle TEXT, prereq_output TEXT, satisfied TEXT, PRIMARY KEY(cycle, name, flow_nums, prereq_name, prereq_cycle, prereq_output)); -CREATE TABLE task_states(name TEXT, cycle TEXT, flow_nums TEXT, time_created TEXT, time_updated TEXT, submit_num INTEGER, status TEXT, flow_wait INTEGER, PRIMARY KEY(name, cycle, flow_nums)); -INSERT INTO task_states VALUES('foo','1','[1]','2022-07-25T16:18:23+01:00','2022-07-25T16:18:23+01:00',1,'preparing',NULL); +CREATE TABLE task_states(name TEXT, cycle TEXT, flow_nums TEXT, time_created TEXT, time_updated TEXT, submit_num INTEGER, status TEXT, flow_wait INTEGER, is_manual_submit INTEGER, PRIMARY KEY(name, cycle, flow_nums)); +INSERT INTO task_states VALUES('foo','1','[1]','2022-07-25T16:18:23+01:00','2022-07-25T16:18:23+01:00',1,'preparing',NULL, '0'); CREATE TABLE task_timeout_timers(cycle TEXT, name TEXT, timeout REAL, PRIMARY KEY(cycle, name)); CREATE TABLE tasks_to_hold(name TEXT, cycle TEXT); CREATE TABLE workflow_flows(flow_num INTEGER, start_time TEXT, description TEXT, PRIMARY KEY(flow_num)); diff --git a/tests/functional/restart/58-waiting-manual-triggered.t b/tests/functional/restart/58-waiting-manual-triggered.t new file mode 100644 index 00000000000..efba9f42b70 --- /dev/null +++ b/tests/functional/restart/58-waiting-manual-triggered.t @@ -0,0 +1,47 @@ +#!/bin/bash +# THIS FILE IS PART OF THE CYLC WORKFLOW ENGINE. +# Copyright (C) NIWA & British Crown (Met Office) & Contributors. +# +# This program is free software: you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation, either version 3 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program. If not, see . + +#------------------------------------------------------------------------------- +# Test that a task manually triggered just before shutdown will run on restart. + +. "$(dirname "$0")/test_header" + +set_test_number 6 + +install_workflow "${TEST_NAME_BASE}" "${TEST_NAME_BASE}" + +run_ok "${TEST_NAME_BASE}-validate" cylc validate "${WORKFLOW_NAME}" + +workflow_run_ok "${TEST_NAME_BASE}-run" cylc play --no-detach "${WORKFLOW_NAME}" + +DB_FILE="${WORKFLOW_RUN_DIR}/log/db" + +# It should have shut down with 2/foo waiting with the is_manual_submit flag on. +TEST_NAME="${TEST_NAME_BASE}-db-task-states" +QUERY='SELECT status, is_manual_submit FROM task_states WHERE cycle IS 2;' +run_ok "$TEST_NAME" sqlite3 "$DB_FILE" "$QUERY" +cmp_ok "${TEST_NAME}.stdout" << '__EOF__' +waiting|1 +__EOF__ + +# It should restart and shut down normally, not stall with 2/foo waiting on 1/foo. +workflow_run_ok "${TEST_NAME_BASE}-restart" cylc play --no-detach "${WORKFLOW_NAME}" +# Check that 2/foo job 02 did run before shutdown. +grep_workflow_log_ok "${TEST_NAME_BASE}-grep" "\[2\/foo running job:02 flows:1\] => succeeded" + +purge +exit diff --git a/tests/functional/restart/58-waiting-manual-triggered/flow.cylc b/tests/functional/restart/58-waiting-manual-triggered/flow.cylc new file mode 100644 index 00000000000..ea5f47c46d7 --- /dev/null +++ b/tests/functional/restart/58-waiting-manual-triggered/flow.cylc @@ -0,0 +1,22 @@ +[scheduler] + [[events]] + stall timeout = PT0S + abort on stall timeout = True +[scheduling] + cycling mode = integer + runahead limit = P1 + final cycle point = 3 + [[graph]] + P1 = foo[-P1] => foo +[runtime] + [[foo]] + script = """ + if (( CYLC_TASK_CYCLE_POINT == 3 )); then + # Order a normal shutdown: no more job submissions, and shut + # down after active jobs (i.e. this one) finish. + cylc stop "$CYLC_WORKFLOW_ID" + # Force-trigger 2/foo before shutdown. On restart it should be + # in the waiting state with the force-triggered flag set. + cylc trigger "${CYLC_WORKFLOW_ID}//2/foo" + fi + """ diff --git a/tests/unit/post_install/test_log_vc_info.py b/tests/unit/post_install/test_log_vc_info.py index cddb8012997..58e16e241b5 100644 --- a/tests/unit/post_install/test_log_vc_info.py +++ b/tests/unit/post_install/test_log_vc_info.py @@ -26,13 +26,15 @@ from cylc.flow.install_plugins.log_vc_info import ( INFO_FILENAME, LOG_VERSION_DIR, - get_diff, _get_git_commit, get_status, get_vc_info, main, + write_diff, ) +from cylc.flow.workflow_files import WorkflowFiles + Fixture = Any @@ -161,12 +163,14 @@ def test_get_vc_info_git(git_source_repo: Tuple[str, str]): @require_git -def test_get_diff_git(git_source_repo: Tuple[str, str]): - """Test get_diff() for a git repo""" - source_dir, commit_sha = git_source_repo - diff = get_diff('git', source_dir) - assert diff is not None - diff_lines = diff.splitlines() +def test_write_diff_git(git_source_repo: Tuple[str, str], tmp_path: Path): + """Test write_diff() for a git repo""" + source_dir, _ = git_source_repo + run_dir = tmp_path / 'run_dir' + (run_dir / WorkflowFiles.LOG_DIR).mkdir(parents=True) + diff_file = write_diff('git', source_dir, run_dir) + diff_lines = diff_file.read_text().splitlines() + assert diff_lines[0].startswith("# Auto-generated diff") for line in ("diff --git a/flow.cylc b/flow.cylc", "- R1 = foo", "+ R1 = bar"): @@ -205,12 +209,14 @@ def test_get_vc_info_svn(svn_source_repo: Tuple[str, str, str]): @require_svn -def test_get_diff_svn(svn_source_repo: Tuple[str, str, str]): - """Test get_diff() for an svn working copy""" - source_dir, uuid, repo_path = svn_source_repo - diff = get_diff('svn', source_dir) - assert diff is not None - diff_lines = diff.splitlines() +def test_write_diff_svn(svn_source_repo: Tuple[str, str, str], tmp_path: Path): + """Test write_diff() for an svn working copy""" + source_dir, _, _ = svn_source_repo + run_dir = tmp_path / 'run_dir' + (run_dir / WorkflowFiles.LOG_DIR).mkdir(parents=True) + diff_file = write_diff('svn', source_dir, run_dir) + diff_lines = diff_file.read_text().splitlines() + assert diff_lines[0].startswith("# Auto-generated diff") for line in (f"--- {source_dir}/flow.cylc (revision 1)", f"+++ {source_dir}/flow.cylc (working copy)", "- R1 = foo", @@ -239,20 +245,26 @@ def test_not_repo(tmp_path: Path, monkeypatch: MonkeyPatch): @require_git def test_no_base_commit_git(tmp_path: Path): - """Test get_vc_info() and get_diff() for a recently init'd git source dir + """Test get_vc_info() and write_diff() for a recently init'd git source dir that does not have a base commit yet.""" source_dir = Path(tmp_path, 'new_git_repo') source_dir.mkdir() subprocess.run(['git', 'init'], cwd=source_dir, check=True) flow_file = source_dir.joinpath('flow.cylc') flow_file.write_text(BASIC_FLOW_1) + run_dir = tmp_path / 'run_dir' + (run_dir / WorkflowFiles.LOG_DIR).mkdir(parents=True) vc_info = get_vc_info(source_dir) assert vc_info is not None - expected = [ + assert list(vc_info.items()) == [ ('version control system', "git"), ('working copy root path', str(source_dir)), ('status', ["?? flow.cylc"]) ] - assert list(vc_info.items()) == expected - assert get_diff('git', source_dir) is None + + # Diff file expected to be empty (only containing comment lines), + # but should work without raising + diff_file = write_diff('git', source_dir, run_dir) + for line in diff_file.read_text().splitlines(): + assert line.startswith('#') diff --git a/tests/unit/test_remote.py b/tests/unit/test_remote.py index 1645665a59f..a1d28cf63e0 100644 --- a/tests/unit/test_remote.py +++ b/tests/unit/test_remote.py @@ -63,11 +63,26 @@ def test_construct_rsync_over_ssh_cmd(): } ) assert host == 'miklegard' - assert ' '.join(cmd) == ( - 'rsync command --delete --rsh=strange_ssh --include=/.service/ ' - '--include=/.service/server.key -a --checksum ' - '--out-format=%o %n%L --no-t --exclude=log --exclude=share ' - '--exclude=work --include=/app/*** --include=/bin/*** ' - '--include=/etc/*** --include=/lib/*** --exclude=* ' - '/foo/ miklegard:/bar/' - ) + assert cmd == [ + 'rsync', + 'command', + '--delete', + '--rsh=strange_ssh', + '--include=/.service/', + '--include=/.service/server.key', + '-a', + '--checksum', + '--out-format=%o %n%L', + '--no-t', + '--exclude=log', + '--exclude=share', + '--exclude=work', + '--include=/ana/***', + '--include=/app/***', + '--include=/bin/***', + '--include=/etc/***', + '--include=/lib/***', + '--exclude=*', + '/foo/', + 'miklegard:/bar/', + ] From 323fa77a5c87a99c42be1c82e67e0a84700732da Mon Sep 17 00:00:00 2001 From: Ronnie Dutta <61982285+MetRonnie@users.noreply.github.com> Date: Mon, 15 Aug 2022 18:41:45 +0100 Subject: [PATCH 42/66] Tidy & type annotations --- cylc/flow/exceptions.py | 7 ++++ cylc/flow/id_cli.py | 2 +- cylc/flow/network/resolvers.py | 40 ++++++++++++--------- cylc/flow/option_parsers.py | 24 +++++++++++++ cylc/flow/scheduler.py | 59 +++++++++++++++--------------- cylc/flow/scripts/message.py | 12 ++++--- cylc/flow/task_events_mgr.py | 65 +++++++++++++++++----------------- cylc/flow/task_message.py | 25 ++++++------- cylc/flow/task_pool.py | 19 +++++++--- 9 files changed, 151 insertions(+), 102 deletions(-) diff --git a/cylc/flow/exceptions.py b/cylc/flow/exceptions.py index 67e2dfa6104..752c5bc1172 100644 --- a/cylc/flow/exceptions.py +++ b/cylc/flow/exceptions.py @@ -115,6 +115,13 @@ class WorkflowEventError(CylcError): class CommandFailedError(CylcError): """Exception for when scheduler commands fail.""" + def __init__(self, value: Union[str, Exception]): + self.value = value + + def __str__(self) -> str: + if isinstance(self.value, Exception): + return f"{type(self.value).__name__}: {self.value}" + return self.value class ServiceFileError(CylcError): diff --git a/cylc/flow/id_cli.py b/cylc/flow/id_cli.py index 2f6bad25db5..96cb438fded 100644 --- a/cylc/flow/id_cli.py +++ b/cylc/flow/id_cli.py @@ -311,7 +311,7 @@ async def parse_ids_async( return workflows, multi_mode -def parse_id(*args, **kwargs): +def parse_id(*args, **kwargs) -> Tuple[str, Optional[Tokens], Any]: return asyncio.run(parse_id_async(*args, **kwargs)) diff --git a/cylc/flow/network/resolvers.py b/cylc/flow/network/resolvers.py index dbb170d3eda..85dd9c64f51 100644 --- a/cylc/flow/network/resolvers.py +++ b/cylc/flow/network/resolvers.py @@ -28,6 +28,7 @@ Dict, Iterable, List, + NamedTuple, Optional, Tuple, TYPE_CHECKING, @@ -36,8 +37,8 @@ from uuid import uuid4 from graphene.utils.str_converters import to_snake_case -from cylc.flow import LOG +from cylc.flow import LOG from cylc.flow.data_store_mgr import ( EDGES, FAMILY_PROXIES, TASK_PROXIES, WORKFLOW, DELTA_ADDED, create_delta_store @@ -59,6 +60,14 @@ from cylc.flow.workflow_status import StopMode +class TaskMsg(NamedTuple): + """Tuple for Scheduler.message_queue""" + job_id: Union[Tokens, str] + event_time: str + severity: Union[str, int] + message: str + + logger = logging.getLogger(__name__) DELTA_SLEEP_INTERVAL = 0.5 @@ -732,35 +741,32 @@ def put_ext_trigger( def put_messages( self, - task_job=None, - event_time=None, - messages=None - ): + task_job: str, + event_time: str, + messages: List[list] + ) -> Tuple[bool, str]: """Put task messages in queue for processing later by the main loop. Arguments: - task_job (str, optional): - Task job in the format ``CYCLE/TASK_NAME/SUBMIT_NUM``. - event_time (str, optional): + task_job: + Job ID in the format ``CYCLE/TASK_NAME/SUBMIT_NUM``. + event_time: Event time as an ISO8601 string. - messages (list, optional): + messages: List in the format ``[[severity, message], ...]``. Returns: - tuple: (outcome, message) - - outcome (bool) + outcome: True if command successfully queued. - message (str) + message: Information about outcome. """ - # TODO: standardise the task_job interface to one of the other - # systems for severity, message in messages: self.schd.message_queue.put( - (task_job, event_time, severity, message)) - return (True, 'Messages queued: %d' % len(messages)) + TaskMsg(task_job, event_time, severity, message) + ) + return (True, f'Messages queued: {len(messages)}') def set_graph_window_extent(self, n_edge_distance): """Set data-store graph window to new max edge distance. diff --git a/cylc/flow/option_parsers.py b/cylc/flow/option_parsers.py index 4bd4ee8797a..a0609e0e172 100644 --- a/cylc/flow/option_parsers.py +++ b/cylc/flow/option_parsers.py @@ -106,6 +106,30 @@ def verbosity_to_log_level(verb: int) -> int: return logging.INFO +def log_level_to_verbosity(lvl: int) -> int: + """Convert log severity level to Cylc verbosity. + + Examples: + >>> log_level_to_verbosity(logging.NOTSET) + 2 + >>> log_level_to_verbosity(logging.DEBUG) + 1 + >>> log_level_to_verbosity(logging.INFO) + 0 + >>> log_level_to_verbosity(logging.WARNING) + -1 + >>> log_level_to_verbosity(logging.ERROR) + -1 + """ + if lvl < logging.DEBUG: + return 2 + if lvl < logging.INFO: + return 1 + if lvl == logging.INFO: + return 0 + return -1 + + def verbosity_to_opts(verb: int) -> List[str]: """Convert Cylc verbosity to the CLI opts required to replicate it. diff --git a/cylc/flow/scheduler.py b/cylc/flow/scheduler.py index f9501eb8d81..79e58c507b6 100644 --- a/cylc/flow/scheduler.py +++ b/cylc/flow/scheduler.py @@ -19,7 +19,6 @@ from contextlib import suppress from collections import deque from dataclasses import dataclass -import logging from optparse import Values import os from pathlib import Path @@ -82,9 +81,14 @@ from cylc.flow.timer import Timer from cylc.flow.network import API from cylc.flow.network.authentication import key_housekeeping +from cylc.flow.network.resolvers import TaskMsg from cylc.flow.network.schema import WorkflowStopMode from cylc.flow.network.server import WorkflowRuntimeServer -from cylc.flow.option_parsers import verbosity_to_env, verbosity_to_opts +from cylc.flow.option_parsers import ( + log_level_to_verbosity, + verbosity_to_env, + verbosity_to_opts, +) from cylc.flow.parsec.exceptions import ParsecError from cylc.flow.parsec.OrderedDict import DictTree from cylc.flow.parsec.validate import DurationFloat @@ -204,7 +208,7 @@ class Scheduler: # queues command_queue: 'Queue[Tuple[str, tuple, dict]]' - message_queue: Queue + message_queue: 'Queue[TaskMsg]' ext_trigger_queue: Queue # configuration @@ -785,24 +789,23 @@ def _load_task_run_times(self, row_idx, row): except (KeyError, ValueError, AttributeError): return - def process_queued_task_messages(self): + def process_queued_task_messages(self) -> None: """Handle incoming task messages for each task proxy.""" - messages = {} + messages: Dict[str, List[Tuple[Optional[int], TaskMsg]]] = {} while self.message_queue.qsize(): try: - task_job, event_time, severity, message = ( - self.message_queue.get(block=False)) + task_msg = self.message_queue.get(block=False) except Empty: break self.message_queue.task_done() - tokens = Tokens(task_job, relative=True) + tokens = Tokens(task_msg.job_id, relative=True) # task ID (job stripped) task_id = tokens.duplicate(job=None).relative_id messages.setdefault(task_id, []) # job may be None (e.g. simulation mode) job = int(tokens['job']) if tokens['job'] else None messages[task_id].append( - (job, event_time, severity, message) + (job, task_msg) ) # Note on to_poll_tasks: If an incoming message is going to cause a # reverse change to task state, it is desirable to confirm this by @@ -813,10 +816,11 @@ def process_queued_task_messages(self): if message_items is None: continue should_poll = False - for submit_num, event_time, severity, message in message_items: + for submit_num, tm in message_items: if self.task_events_mgr.process_message( - itask, severity, message, event_time, - self.task_events_mgr.FLAG_RECEIVED, submit_num): + itask, tm.severity, tm.message, tm.event_time, + self.task_events_mgr.FLAG_RECEIVED, submit_num + ): should_poll = True if should_poll: to_poll_tasks.append(itask) @@ -841,7 +845,7 @@ def process_command_queue(self) -> None: LOG.info(f"Processing {qsize} queued command(s)") while True: try: - command = (self.command_queue.get(False)) + command = self.command_queue.get(False) name, args, kwargs = command except Empty: break @@ -948,15 +952,15 @@ def command_resume(self) -> None: """Resume paused workflow.""" self.resume_workflow() - def command_poll_tasks(self, items: List[str]): + def command_poll_tasks(self, items: List[str]) -> int: """Poll pollable tasks or a task or family if options are provided.""" if self.config.run_mode('simulation'): - return + return 0 itasks, _, bad_items = self.pool.filter_task_proxies(items) self.task_job_mgr.poll_task_jobs(self.workflow, itasks) return len(bad_items) - def command_kill_tasks(self, items: List[str]): + def command_kill_tasks(self, items: List[str]) -> int: """Kill all tasks or a task/family if options are provided.""" itasks, _, bad_items = self.pool.filter_task_proxies(items) if self.config.run_mode('simulation'): @@ -987,23 +991,16 @@ def command_pause(self) -> None: self.pause_workflow() @staticmethod - def command_set_verbosity(lvl): + def command_set_verbosity(lvl: Union[int, str]) -> None: """Set workflow verbosity.""" try: - LOG.setLevel(int(lvl)) - except (TypeError, ValueError): - return - if lvl <= logging.DEBUG: - cylc.flow.flags.verbosity = 2 - elif lvl < logging.INFO: - cylc.flow.flags.verbosity = 1 - elif lvl == logging.INFO: - cylc.flow.flags.verbosity = 0 - else: - cylc.flow.flags.verbosity = -1 - return True, 'OK' + lvl = int(lvl) + LOG.setLevel(lvl) + except (TypeError, ValueError) as exc: + raise CommandFailedError(exc) + cylc.flow.flags.verbosity = log_level_to_verbosity(lvl) - def command_remove_tasks(self, items): + def command_remove_tasks(self, items) -> int: """Remove tasks.""" return self.pool.remove_tasks(items) @@ -1018,7 +1015,7 @@ def command_reload_workflow(self) -> None: try: self.load_flow_file(is_reload=True) except (ParsecError, CylcConfigError) as exc: - raise CommandFailedError(f"{type(exc).__name__}: {exc}") + raise CommandFailedError(exc) self.broadcast_mgr.linearized_ancestors = ( self.config.get_linearized_ancestors()) self.pool.set_do_reload(self.config) diff --git a/cylc/flow/scripts/message.py b/cylc/flow/scripts/message.py index c7d3606f199..9e9e1f258ef 100755 --- a/cylc/flow/scripts/message.py +++ b/cylc/flow/scripts/message.py @@ -95,7 +95,6 @@ def get_option_parser() -> COP: __doc__, comms=True, argdoc=[ - # TODO COP.optional(WORKFLOW_ID_ARG_DOC), COP.optional( ('JOB', 'Job ID - CYCLE/TASK_NAME/SUBMIT_NUM') @@ -129,10 +128,15 @@ def main(parser: COP, options: 'Values', *args: str) -> None: # (As of Dec 2020 some functional tests still use the classic # two arg interface) workflow_id = os.getenv('CYLC_WORKFLOW_ID') - task_job = os.getenv('CYLC_TASK_JOB') + job_id = os.getenv('CYLC_TASK_JOB') + if not workflow_id or not job_id: + raise InputError( + "Must set $CYLC_WORKFLOW_ID and $CYLC_TASK_JOB if not " + "specified as arguments" + ) message_strs = list(args) else: - workflow_id, task_job, *message_strs = args + workflow_id, job_id, *message_strs = args workflow_id, *_ = parse_id( workflow_id, constraint='workflows', @@ -171,4 +175,4 @@ def main(parser: COP, options: 'Values', *args: str) -> None: messages.append([options.severity, message_str.strip()]) else: messages.append([getLevelName(INFO), message_str.strip()]) - record_messages(workflow_id, task_job, messages) + record_messages(workflow_id, job_id, messages) diff --git a/cylc/flow/task_events_mgr.py b/cylc/flow/task_events_mgr.py index b0386ccc292..9b40d419a91 100644 --- a/cylc/flow/task_events_mgr.py +++ b/cylc/flow/task_events_mgr.py @@ -33,13 +33,12 @@ from shlex import quote import shlex from time import time -from typing import TYPE_CHECKING - -from cylc.flow.parsec.config import ItemNotFoundError +from typing import TYPE_CHECKING, Optional, Union, cast from cylc.flow import LOG, LOG_LEVELS from cylc.flow.cfgspec.glbl_cfg import glbl_cfg from cylc.flow.hostuserutil import get_host, get_user, is_remote_platform +from cylc.flow.parsec.config import ItemNotFoundError from cylc.flow.pathutil import ( get_remote_workflow_run_job_dir, get_workflow_run_job_dir) @@ -523,13 +522,13 @@ def process_events(self, schd: 'Scheduler') -> None: def process_message( self, - itask, - severity, - message, - event_time=None, - flag=FLAG_INTERNAL, - submit_num=None, - ): + itask: 'TaskProxy', + severity: Union[str, int], + message: str, + event_time: Optional[str] = None, + flag: str = FLAG_INTERNAL, + submit_num: Optional[int] = None, + ) -> Optional[bool]: """Parse a task message and update task state. Incoming, e.g. "succeeded at