From 28ed33057fbc726f08f24395cbe3660735e8804f Mon Sep 17 00:00:00 2001 From: Tim Pillinger <26465611+wxtim@users.noreply.github.com> Date: Tue, 24 May 2022 12:35:21 +0100 Subject: [PATCH 1/4] Added a noqa tag to a new flake8-simplify case. In this case the suggested refactor would IMO make the code less rather than more readable. --- cylc/flow/id_match.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cylc/flow/id_match.py b/cylc/flow/id_match.py index e6ea8ae7c6..9f0e116cac 100644 --- a/cylc/flow/id_match.py +++ b/cylc/flow/id_match.py @@ -167,7 +167,7 @@ def filter_ids( break # filter by task - elif lowest_token == IDTokens.Task: + elif lowest_token == IDTokens.Task: # noqa SIM106 cycle = tokens[IDTokens.Cycle.value] cycle_sel_raw = tokens.get(IDTokens.Cycle.value + '_sel') cycle_sel = cycle_sel_raw or '*' From bdd609fc070e01241cb6efdff3a497f8aaae04f5 Mon Sep 17 00:00:00 2001 From: Tim Pillinger <26465611+wxtim@users.noreply.github.com> Date: Tue, 24 May 2022 12:30:40 +0100 Subject: [PATCH 2/4] Allow setting of default batch system directives in the platform config --- CHANGES.md | 5 ++ cylc/flow/cfgspec/globalcfg.py | 11 +++ cylc/flow/task_job_mgr.py | 25 ++++++- .../platforms/09-default-directives.t | 70 +++++++++++++++++++ 4 files changed, 110 insertions(+), 1 deletion(-) create mode 100755 tests/functional/platforms/09-default-directives.t diff --git a/CHANGES.md b/CHANGES.md index ce24798ac1..3c76ecacf4 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -33,6 +33,11 @@ ones in. --> Fourth Release Candidate for Cylc 8 suitable for acceptance testing. +### Enhancements + +[#4896](https://github.com/cylc/cylc-flow/pull/4896) - Allow the setting of +default job runner directives for plaforms. + ### Fixes [#4881](https://github.com/cylc/cylc-flow/pull/4881) - Fix bug where commands diff --git a/cylc/flow/cfgspec/globalcfg.py b/cylc/flow/cfgspec/globalcfg.py index 3471afa1ba..33153f8012 100644 --- a/cylc/flow/cfgspec/globalcfg.py +++ b/cylc/flow/cfgspec/globalcfg.py @@ -1308,6 +1308,17 @@ be appropriate if following the pattern ``hosts = main, backup, failsafe``. ''') + with Conf('directives', desc=''' + Job runner (batch scheduler) directives. + + Default directives for :cylc:conf: + `flow.cylc[runtime][][directives]` + '''): + Conf('', VDR.V_STRING, desc=''' + Example directives for the built-in job runner handlers + are shown in :ref:`AvailableMethods`. + ''') + with Conf('localhost', meta=Platform, desc=''' A default platform defining settings for jobs to be run on the same host as the workflow scheduler. diff --git a/cylc/flow/task_job_mgr.py b/cylc/flow/task_job_mgr.py index 007afc0979..ab4174b555 100644 --- a/cylc/flow/task_job_mgr.py +++ b/cylc/flow/task_job_mgr.py @@ -1278,6 +1278,27 @@ def _prep_submit_task_job_impl(self, workflow, itask, rtconfig): job_d=job_d ) + @staticmethod + def default_dict_merger(defaults, tdef): + """Use directive from platform if not set in rtconfig. + + For item in defaults add item to output if not in tdef. + + Examples: + >>> defaults = {'use': 1, 'override': 1} + >>> tdef = {'override': 2, 'nodefault': 1} + >>> result = TaskJobManager.default_dict_merger(defaults, tdef) + >>> {i: result[i] for i in sorted(result)} + {'nodefault': 1, 'override': 2, 'use': 1} + """ + output = {} + for key in set(list(defaults.keys()) + list(tdef.keys())): + if key in defaults and key not in tdef: + output[key] = defaults[key] + else: + output[key] = tdef[key] + return output + def get_job_conf( self, workflow, @@ -1299,7 +1320,9 @@ def get_job_conf( itask.platform['job runner command template'] ), 'dependencies': itask.state.get_resolved_dependencies(), - 'directives': rtconfig['directives'], + 'directives': self.default_dict_merger( + itask.platform['directives'], rtconfig['directives'] + ), 'environment': rtconfig['environment'], 'execution_time_limit': itask.summary[self.KEY_EXECUTE_TIME_LIMIT], 'env-script': rtconfig['env-script'], diff --git a/tests/functional/platforms/09-default-directives.t b/tests/functional/platforms/09-default-directives.t new file mode 100755 index 0000000000..c392186a01 --- /dev/null +++ b/tests/functional/platforms/09-default-directives.t @@ -0,0 +1,70 @@ +#!/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 . +#------------------------------------------------------------------------------- +# Platforms can have default directives and these are overridden as expected. +export REQUIRE_PLATFORM="runner:slurm" +. "$(dirname "$0")/test_header" + +set_test_number 5 + +create_test_global_config "" " + [platforms] + [[no_default, default_only, overridden, neither]] + hosts = localhost + job runner = slurm + [[default_only, overridden]] + [[[directives]]] + --wurble=foo +" + +init_workflow "${TEST_NAME_BASE}" <<'__FLOW_CONF__' +[scheduler] + [[events]] + stall timeout = PT0S +[scheduling] + [[graph]] + R1 = lewis & morse & barnaby & cadfael + +[runtime] + [[lewis]] + platform = no_default + [[[directives]]] + --wurble=bar + [[morse]] + platform = default_only + [[barnaby]] + platform = overridden + [[[directives]]] + --wurble=qux + [[cadfael]] + platform = neither + +__FLOW_CONF__ + +run_fail "${TEST_NAME_BASE}-play" \ + cylc play "${WORKFLOW_NAME}" --no-detach + +LOG_DIR="${RUN_DIR}/${WORKFLOW_NAME}/log/job/1/" + +named_grep_ok "${TEST_NAME_BASE}-no-default" "--wurble=bar" "${LOG_DIR}/lewis/NN/job" +named_grep_ok "${TEST_NAME_BASE}-default-only" "--wurble=foo" "${LOG_DIR}/morse/NN/job" +named_grep_ok "${TEST_NAME_BASE}-overridden" "--wurble=qux" "${LOG_DIR}/barnaby/NN/job" +grep_fail "--wurble" "${LOG_DIR}/cadfael/NN/job" + +purge +exit 0 + From fc67717f3ab6e8b72071ebb033a7d56059ed4b48 Mon Sep 17 00:00:00 2001 From: Tim Pillinger <26465611+wxtim@users.noreply.github.com> Date: Wed, 25 May 2022 14:36:20 +0100 Subject: [PATCH 3/4] Apply suggestions from code review Co-authored-by: Ronnie Dutta <61982285+MetRonnie@users.noreply.github.com> --- CHANGES.md | 2 +- cylc/flow/cfgspec/globalcfg.py | 6 +++--- cylc/flow/task_job_mgr.py | 27 +++------------------------ 3 files changed, 7 insertions(+), 28 deletions(-) diff --git a/CHANGES.md b/CHANGES.md index e66f12d50d..dbb32a6926 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -36,7 +36,7 @@ Fourth Release Candidate for Cylc 8 suitable for acceptance testing. ### Enhancements [#4896](https://github.com/cylc/cylc-flow/pull/4896) - Allow the setting of -default job runner directives for plaforms. +default job runner directives for platforms. [#4887](https://github.com/cylc/cylc-flow/pull/4887) - Disallow relative paths in `global.cylc[install]source dirs`. diff --git a/cylc/flow/cfgspec/globalcfg.py b/cylc/flow/cfgspec/globalcfg.py index fb13e28071..8346140706 100644 --- a/cylc/flow/cfgspec/globalcfg.py +++ b/cylc/flow/cfgspec/globalcfg.py @@ -1314,10 +1314,10 @@ ``hosts = main, backup, failsafe``. ''') with Conf('directives', desc=''' - Job runner (batch scheduler) directives. - - Default directives for :cylc:conf: + :Defaults for: :cylc:conf: `flow.cylc[runtime][][directives]` + + Job runner (batch scheduler) directives. '''): Conf('', VDR.V_STRING, desc=''' Example directives for the built-in job runner handlers diff --git a/cylc/flow/task_job_mgr.py b/cylc/flow/task_job_mgr.py index ab4174b555..47dde478fa 100644 --- a/cylc/flow/task_job_mgr.py +++ b/cylc/flow/task_job_mgr.py @@ -1278,27 +1278,6 @@ def _prep_submit_task_job_impl(self, workflow, itask, rtconfig): job_d=job_d ) - @staticmethod - def default_dict_merger(defaults, tdef): - """Use directive from platform if not set in rtconfig. - - For item in defaults add item to output if not in tdef. - - Examples: - >>> defaults = {'use': 1, 'override': 1} - >>> tdef = {'override': 2, 'nodefault': 1} - >>> result = TaskJobManager.default_dict_merger(defaults, tdef) - >>> {i: result[i] for i in sorted(result)} - {'nodefault': 1, 'override': 2, 'use': 1} - """ - output = {} - for key in set(list(defaults.keys()) + list(tdef.keys())): - if key in defaults and key not in tdef: - output[key] = defaults[key] - else: - output[key] = tdef[key] - return output - def get_job_conf( self, workflow, @@ -1320,9 +1299,9 @@ def get_job_conf( itask.platform['job runner command template'] ), 'dependencies': itask.state.get_resolved_dependencies(), - 'directives': self.default_dict_merger( - itask.platform['directives'], rtconfig['directives'] - ), + 'directives': { + **itask.platform['directives'], **rtconfig['directives'] + }, 'environment': rtconfig['environment'], 'execution_time_limit': itask.summary[self.KEY_EXECUTE_TIME_LIMIT], 'env-script': rtconfig['env-script'], From d78a113648311aafd30379c1697ba726585447f9 Mon Sep 17 00:00:00 2001 From: Tim Pillinger <26465611+wxtim@users.noreply.github.com> Date: Fri, 27 May 2022 08:03:37 +0100 Subject: [PATCH 4/4] Update cylc/flow/cfgspec/globalcfg.py Co-authored-by: Ronnie Dutta <61982285+MetRonnie@users.noreply.github.com> --- cylc/flow/cfgspec/globalcfg.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cylc/flow/cfgspec/globalcfg.py b/cylc/flow/cfgspec/globalcfg.py index 8346140706..f14d6c4c16 100644 --- a/cylc/flow/cfgspec/globalcfg.py +++ b/cylc/flow/cfgspec/globalcfg.py @@ -1314,8 +1314,8 @@ ``hosts = main, backup, failsafe``. ''') with Conf('directives', desc=''' - :Defaults for: :cylc:conf: - `flow.cylc[runtime][][directives]` + :Defaults for: :cylc:conf:`flow.cylc\ + [runtime][][directives]` Job runner (batch scheduler) directives. '''):