diff --git a/CHANGES.md b/CHANGES.md index f330ff8121c..4a98864a2a2 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -54,6 +54,10 @@ default job runner directives for platforms. [#4975](https://github.com/cylc/cylc-flow/pull/4975) - Fix selection of platforms from `[job]` and `[remote]` configs. +[#4948](https://github.com/cylc/cylc-flow/pull/4948) - Fix lack of +errors/warnings for deprecated `[runtime][][remote]retrieve job logs *` +settings. + [#4970](https://github.com/cylc/cylc-flow/pull/4970) - Fix handling of suicide triggers in back-compat mode. diff --git a/cylc/flow/cfgspec/workflow.py b/cylc/flow/cfgspec/workflow.py index 39ec3cb1257..9816c2a32f4 100644 --- a/cylc/flow/cfgspec/workflow.py +++ b/cylc/flow/cfgspec/workflow.py @@ -59,15 +59,14 @@ # Regex to check whether a string is a command REC_COMMAND = re.compile(r'(`|\$\()\s*(.*)\s*([`)])$') -# Cylc8 Deprecation note. REPLACED_BY_PLATFORMS = ''' -.. warning:: +.. deprecated:: 8.0.0 - .. deprecated:: 8.0.0 - - This will be removed in a future version of Cylc 8. - - Use :cylc:conf:`flow.cylc[runtime][]platform` instead. + This is used to select a matching platform. + It will be removed in a future version of Cylc 8. + Please set a suitable platform in + :cylc:conf:`flow.cylc[runtime][]platform` instead. + :ref:`See the migration guide `. ''' @@ -214,11 +213,12 @@ def get_script_common_text(this: str, example: Optional[str] = None): ''') Conf('install', VDR.V_STRING_LIST, desc=''' - Configure directories and files to be installed on remote hosts. + Configure custom directories and files to be installed on remote + hosts. .. note:: - The following directories are installed by default: + The following directories already get installed by default: * ``app/`` * ``bin/`` @@ -1303,48 +1303,40 @@ def get_script_common_text(this: str, example: Optional[str] = None): excluded by omission from an ``include`` list. ''') - with Conf('job', desc=dedent(''' - .. deprecated:: 8.0.0 - + with Conf('job', desc=REPLACED_BY_PLATFORMS + dedent(''' This section configures the means by which cylc submits task job scripts to run. - - ''') + REPLACED_BY_PLATFORMS): + ''')): Conf('batch system', VDR.V_STRING, desc=''' - Batch/Queuing system to submit task jobs to. - .. deprecated:: 8.0.0 Kept for back compatibility but replaced by :cylc:conf:`global.cylc[platforms][] job runner`. + + Batch/queuing system (aka job runner) to submit task + jobs to. ''') Conf('batch submit command template', VDR.V_STRING, desc=''' - Override the default job submission command for the chosen - batch system. - - .. seealso:: + .. deprecated:: 8.0.0 Kept for back compatibility but replaced by :cylc:conf:`global.cylc[platforms][] job runner command template`. - ''') - with Conf('remote', desc=dedent(''' - .. deprecated:: 8.0.0 + Override the default job submission command for the chosen + batch system. + ''') - ''') + REPLACED_BY_PLATFORMS): + with Conf('remote', desc=REPLACED_BY_PLATFORMS): Conf('host', VDR.V_STRING, desc=REPLACED_BY_PLATFORMS) - # TODO: Convert URL to a stable or latest release doc after 8.0 - # https://github.com/cylc/cylc-flow/issues/4663 Conf('owner', VDR.V_STRING, desc=""" - This setting is obsolete at Cylc 8. + .. warning:: - .. seealso:: + This setting is obsolete at Cylc 8. - `Documentation on changes to remote owner - `_ + See :ref:`documentation on changes to remote owner + <728.remote_owner>` """) Conf('retrieve job logs', VDR.V_BOOLEAN, desc=REPLACED_BY_PLATFORMS) diff --git a/cylc/flow/config.py b/cylc/flow/config.py index 74e8f31c62e..ce879dd8b1a 100644 --- a/cylc/flow/config.py +++ b/cylc/flow/config.py @@ -1306,9 +1306,11 @@ def configure_sim_modes(self): # Dummy mode jobs should run on platform localhost # All Cylc 7 config items which conflict with platform are removed. - for section, key, _ in FORBIDDEN_WITH_PLATFORM: - if (section in rtc and key in rtc[section]): - rtc[section][key] = None + for section, keys in FORBIDDEN_WITH_PLATFORM.items(): + if section in rtc: + for key in keys: + if key in rtc[section]: + rtc[section][key] = None rtc['platform'] = 'localhost' @@ -2083,8 +2085,9 @@ def _proc_triggers(self, parser, seq, task_triggers): if suicides and not cylc.flow.flags.cylc7_back_compat: LOG.warning( - f"{suicides} suicide triggers detected. These are rarely" - " needed in Cylc 8 - have you upgraded from Cylc 7 syntax?" + f"{suicides} suicide trigger(s) detected. These are rarely " + "needed in Cylc 8 - see https://cylc.github.io/cylc-doc/" + "latest/html/7-to-8/major-changes/suicide-triggers.html" ) def set_required_outputs( diff --git a/cylc/flow/platforms.py b/cylc/flow/platforms.py index d915a2461d2..02eb123ae7c 100644 --- a/cylc/flow/platforms.py +++ b/cylc/flow/platforms.py @@ -20,7 +20,7 @@ import re from copy import deepcopy from typing import ( - Any, Dict, Iterable, List, Optional, Tuple, Set, Union, overload + TYPE_CHECKING, Any, Dict, Iterable, List, Optional, Set, Union, overload ) from cylc.flow import LOG @@ -31,13 +31,24 @@ from cylc.flow.cfgspec.glbl_cfg import glbl_cfg from cylc.flow.hostuserutil import is_remote_host +if TYPE_CHECKING: + from cylc.flow.parsec.OrderedDict import OrderedDictWithDefaults + UNKNOWN_TASK = 'unknown task' -FORBIDDEN_WITH_PLATFORM: Tuple[Tuple[str, str, List[Optional[str]]], ...] = ( - ('remote', 'host', ['localhost', None]), - ('job', 'batch system', [None]), - ('job', 'batch submit command template', [None]) -) +FORBIDDEN_WITH_PLATFORM: Dict[str, Dict[str, Set[Optional[str]]]] = { + 'remote': { + # setting: exclusions + 'host': {'localhost', None}, + 'retrieve job logs': {None}, + 'retrieve job logs max size': {None}, + 'retrieve job logs retry delays': {None}, + }, + 'job': { + 'batch system': {None}, + 'batch submit command template': {None} + } +} DEFAULT_JOB_RUNNER = 'background' SINGLE_HOST_JOB_RUNNERS = ['background', 'at'] @@ -69,7 +80,7 @@ def log_platform_event( @overload def get_platform( task_conf: Optional[str] = None, - task_id: str = UNKNOWN_TASK, + task_name: str = UNKNOWN_TASK, bad_hosts: Optional[Set[str]] = None ) -> Dict[str, Any]: ... @@ -77,8 +88,8 @@ def get_platform( @overload def get_platform( - task_conf: Dict[str, Any], - task_id: str = UNKNOWN_TASK, + task_conf: Union[dict, 'OrderedDictWithDefaults'], + task_name: str = UNKNOWN_TASK, bad_hosts: Optional[Set[str]] = None ) -> Optional[Dict[str, Any]]: ... @@ -93,8 +104,8 @@ def get_platform( # remove at: # Cylc8.x def get_platform( - task_conf: Union[str, Dict[str, Any], None] = None, - task_id: str = UNKNOWN_TASK, + task_conf: Union[str, dict, 'OrderedDictWithDefaults', None] = None, + task_name: str = UNKNOWN_TASK, bad_hosts: Optional[Set[str]] = None ) -> Optional[Dict[str, Any]]: """Get a platform. @@ -105,21 +116,22 @@ def get_platform( Args: task_conf: If str this is assumed to be the platform name, otherwise this should be a configuration for a task. - task_id: Task identification string - help produce more helpful error - messages. + task_name: Help produce more helpful error messages. Returns: platform: A platform definition dictionary. Uses either - get_platform() or platform_from_job_info(), but to the + get_platform() or platform_name_from_job_info(), but to the user these look the same. """ if task_conf is None or isinstance(task_conf, str): # noqa: SIM 114 # task_conf is a platform name, or get localhost if None return platform_from_name(task_conf, bad_hosts=bad_hosts) + # NOTE: Do NOT use .get() on OrderedDictWithDefaults - + # https://github.com/cylc/cylc-flow/pull/4975 elif 'platform' in task_conf and task_conf['platform']: # Check whether task has conflicting Cylc7 items. - fail_if_platform_and_host_conflict(task_conf, task_id) + fail_if_platform_and_host_conflict(task_conf, task_name) if is_platform_definition_subshell(task_conf['platform']): # Platform definition is using subshell e.g. platform = $(foo); @@ -136,13 +148,12 @@ def get_platform( return platform_from_name() else: # Need to calculate platform - task_job_section: Dict[Any, Any] = {} - task_remote_section: Dict[Any, Any] = {} + # NOTE: Do NOT use .get() on OrderedDictWithDefaults - see above task_job_section = task_conf['job'] if 'job' in task_conf else {} task_remote_section = ( task_conf['remote'] if 'remote' in task_conf else {}) return platform_from_name( - platform_from_job_info( + platform_name_from_job_info( glbl_cfg(cached=False).get(['platforms']), task_job_section, task_remote_section @@ -240,7 +251,7 @@ def platform_from_name( def get_platform_from_group( - group: Dict[str, Any], + group: Union[dict, 'OrderedDictWithDefaults'], group_name: str, bad_hosts: Optional[Set[str]] = None ) -> str: @@ -288,8 +299,8 @@ def get_platform_from_group( return HOST_SELECTION_METHODS[method](platform_names) -def platform_from_job_info( - platforms: Dict[str, Any], +def platform_name_from_job_info( + platforms: Union[dict, 'OrderedDictWithDefaults'], job: Dict[str, Any], remote: Dict[str, Any] ) -> str: @@ -364,14 +375,14 @@ def platform_from_job_info( ... } >>> job = {'batch system': 'slurm'} >>> remote = {'host': 'localhost'} - >>> platform_from_job_info(platforms, job, remote) + >>> platform_name_from_job_info(platforms, job, remote) 'sugar' >>> remote = {} - >>> platform_from_job_info(platforms, job, remote) + >>> platform_name_from_job_info(platforms, job, remote) 'sugar' >>> remote ={'host': 'desktop92'} >>> job = {} - >>> platform_from_job_info(platforms, job, remote) + >>> platform_name_from_job_info(platforms, job, remote) 'desktop92' """ @@ -380,6 +391,9 @@ def platform_from_job_info( # - In the case of "host" we also want a regex match to the platform name # - In the case of "batch system" we want to match the name of the # system/job runner to a platform when host is localhost. + + # NOTE: Do NOT use .get() on OrderedDictWithDefaults - + # https://github.com/cylc/cylc-flow/pull/4975 if 'host' in remote and remote['host']: task_host = remote['host'] else: @@ -389,6 +403,7 @@ def platform_from_job_info( else: # Necessary? Perhaps not if batch system default is 'background' task_job_runner = 'background' + # Riffle through the platforms looking for a match to our task settings. # reverse dict order so that user config platforms added last are examined # before site config platforms. @@ -509,42 +524,44 @@ def get_host_from_platform( return HOST_SELECTION_METHODS[method](goodhosts) -def fail_if_platform_and_host_conflict(task_conf, task_name): - """Raise an error if task spec contains platform and forbidden host items. +def fail_if_platform_and_host_conflict( + task_conf: Union[dict, 'OrderedDictWithDefaults'], task_name: str +) -> None: + """Raise an error if [runtime][] spec contains platform and + forbidden host items. Args: - task_conf(dict, OrderedDictWithDefaults): - A specification to be checked. - task_name(string): - A name to be given in an error. + task_conf: A specification to be checked. + task_name: A name to be given in an error. Raises: PlatformLookupError - if platform and host items conflict """ + # NOTE: Do NOT use .get() on OrderedDictWithDefaults - + # https://github.com/cylc/cylc-flow/pull/4975 if 'platform' in task_conf and task_conf['platform']: - fail_items = '' - for section, key, _ in FORBIDDEN_WITH_PLATFORM: + fail_items = [ + f'\n * [{section}]{key}' + for section, keys in FORBIDDEN_WITH_PLATFORM.items() + if section in task_conf + for key, _ in keys.items() if ( - section in task_conf and key in task_conf[section] and task_conf[section][key] is not None - ): - fail_items += ( - f' * platform = {task_conf["platform"]} AND' - f' [{section}]{key} = {task_conf[section][key]}\n' - ) - if fail_items != '': + ) + ] + if fail_items: raise PlatformLookupError( - f"A mixture of Cylc 7 (host) and Cylc 8 (platform) " - f"logic should not be used. In this case the task " - f"\"{task_name}\" has the following settings which " - f"are not compatible:\n{fail_items}" + f"Task '{task_name}' has the following deprecated '[runtime]' " + "setting(s) which cannot be used with " + f"'platform = {task_conf['platform']}':{''.join(fail_items)}" ) def get_platform_deprecated_settings( - task_conf: Dict[str, Any], task_name: str = UNKNOWN_TASK + task_conf: Union[dict, 'OrderedDictWithDefaults'], + task_name: str = UNKNOWN_TASK ) -> List[str]: """Return deprecated [runtime][] settings that should be upgraded to platforms. @@ -553,18 +570,16 @@ def get_platform_deprecated_settings( task_conf: Runtime configuration for the task. task_name: The task name. """ - result: List[str] = [] - for section, key, exceptions in FORBIDDEN_WITH_PLATFORM: + return [ + f'[runtime][{task_name}][{section}]{key} = {task_conf[section][key]}' + for section, keys in FORBIDDEN_WITH_PLATFORM.items() + if section in task_conf + for key, exclusions in keys.items() if ( - section in task_conf and key in task_conf[section] and - task_conf[section][key] not in exceptions - ): - result.append( - f'[runtime][{task_name}][{section}]{key} = ' - f'{task_conf[section][key]}' - ) - return result + task_conf[section][key] not in exclusions + ) + ] def is_platform_definition_subshell(value: str) -> bool: @@ -664,7 +679,9 @@ def get_localhost_install_target() -> str: return get_install_target_from_platform(localhost) -def _validate_single_host(platforms_cfg) -> None: +def _validate_single_host( + platforms_cfg: Union[dict, 'OrderedDictWithDefaults'] +) -> None: """Check that single-host platforms only specify a single host. Some job runners don't work across multiple hosts; the job ID is only valid @@ -672,6 +689,8 @@ def _validate_single_host(platforms_cfg) -> None: """ bad_platforms = [] runners = set() + name: str + config: dict for name, config in platforms_cfg.items(): runner = config.get('job runner', DEFAULT_JOB_RUNNER) hosts = config.get('hosts', []) diff --git a/cylc/flow/task_job_mgr.py b/cylc/flow/task_job_mgr.py index 3b63a4446b1..c09781b9eca 100644 --- a/cylc/flow/task_job_mgr.py +++ b/cylc/flow/task_job_mgr.py @@ -36,6 +36,7 @@ ) from shutil import rmtree from time import time +from typing import TYPE_CHECKING from cylc.flow import LOG from cylc.flow.job_runner_mgr import JobPollContext @@ -112,6 +113,9 @@ ) from cylc.flow.cfgspec.globalcfg import SYSPATH +if TYPE_CHECKING: + from cylc.flow.task_proxy import TaskProxy + class TaskJobManager: """Manage task job submit, poll and kill. @@ -1076,7 +1080,12 @@ def _submit_task_job_callback(self, workflow, itask, cmd_ctx, line): itask, CRITICAL, self.task_events_mgr.EVENT_SUBMIT_FAILED, ctx.timestamp) - def _prep_submit_task_job(self, workflow, itask, check_syntax=True): + def _prep_submit_task_job( + self, + workflow: str, + itask: 'TaskProxy', + check_syntax: bool = True + ): """Prepare a task job submission. Return itask on a good preparation. @@ -1167,7 +1176,9 @@ def _prep_submit_task_job(self, workflow, itask, check_syntax=True): rtconfig['remote']['host'] = host_n try: - platform = get_platform(rtconfig, bad_hosts=self.bad_hosts) + platform = get_platform( + rtconfig, itask.tdef.name, bad_hosts=self.bad_hosts + ) except PlatformLookupError as exc: # Submit number not yet incremented diff --git a/tests/functional/events/11-cycle-task-event-job-logs-retrieve/flow.cylc b/tests/functional/events/11-cycle-task-event-job-logs-retrieve/flow.cylc index 68554599b11..e3a76285f42 100644 --- a/tests/functional/events/11-cycle-task-event-job-logs-retrieve/flow.cylc +++ b/tests/functional/events/11-cycle-task-event-job-logs-retrieve/flow.cylc @@ -15,9 +15,9 @@ [runtime] [[T]] script = test "${CYLC_TASK_TRY_NUMBER}" -eq 3 - platform = {{ environ['CYLC_TEST_PLATFORM'] }} - [[[job]]] - execution retry delays = PT0S, 2*PT1S + execution retry delays = PT0S, 2*PT1S + [[[remote]]] + host = {{ environ['CYLC_TEST_HOST'] }} [[t1]] inherit = T [[[remote]]] diff --git a/tests/functional/events/32-task-event-job-logs-retrieve-2.t b/tests/functional/events/32-task-event-job-logs-retrieve-2.t index 4a20164b37a..4ca3243b64c 100755 --- a/tests/functional/events/32-task-event-job-logs-retrieve-2.t +++ b/tests/functional/events/32-task-event-job-logs-retrieve-2.t @@ -16,17 +16,23 @@ # along with this program. If not, see . #------------------------------------------------------------------------------- # Test remote job logs retrieval OK with only "job.out" on a succeeded task. -export REQUIRE_PLATFORM='loc:remote' +export REQUIRE_PLATFORM='loc:remote fs:indep comms:tcp' . "$(dirname "$0")/test_header" set_test_number 5 +create_test_global_config "" " +[platforms] + [[treadstone]] + hosts = ${CYLC_TEST_HOST} + install target = ${CYLC_TEST_INSTALL_TARGET} + retrieve job logs = True +" + install_workflow "${TEST_NAME_BASE}" "${TEST_NAME_BASE}" -run_ok "${TEST_NAME_BASE}-validate" \ - cylc validate -s "PLATFORM='${CYLC_TEST_PLATFORM}'" "${WORKFLOW_NAME}" +run_ok "${TEST_NAME_BASE}-validate" cylc validate "${WORKFLOW_NAME}" workflow_run_ok "${TEST_NAME_BASE}-run" \ - cylc play --reference-test --debug --no-detach \ - -s "PLATFORM='${CYLC_TEST_PLATFORM}'" "${WORKFLOW_NAME}" + cylc play --reference-test -vv --no-detach "${WORKFLOW_NAME}" sed "/'job-logs-retrieve'/!d" \ "${WORKFLOW_RUN_DIR}/log/job/1/t1/01/job-activity.log" \ @@ -38,4 +44,3 @@ exists_ok "${WORKFLOW_RUN_DIR}/log/job/1/t1/01/job.out" exists_fail "${WORKFLOW_RUN_DIR}/log/job/1/t1/01/job.err" purge -exit diff --git a/tests/functional/events/32-task-event-job-logs-retrieve-2/flow.cylc b/tests/functional/events/32-task-event-job-logs-retrieve-2/flow.cylc index e93b5d165ab..e3ea1a0c3df 100644 --- a/tests/functional/events/32-task-event-job-logs-retrieve-2/flow.cylc +++ b/tests/functional/events/32-task-event-job-logs-retrieve-2/flow.cylc @@ -1,14 +1,11 @@ -#!jinja2 [meta] - title=Task Event Job Log Retrieve 1 + title = Task Event Job Log Retrieve 1 [scheduling] [[graph]] - R1=t1 + R1 = t1 [runtime] [[t1]] - script=rm -f "${CYLC_TASK_LOG_ROOT}.err" - platform = {{ PLATFORM }} - [[[remote]]] - retrieve job logs = True + script = rm -f "${CYLC_TASK_LOG_ROOT}.err" + platform = treadstone diff --git a/tests/functional/events/33-task-event-job-logs-retrieve-3.t b/tests/functional/events/33-task-event-job-logs-retrieve-3.t index 797f285aaa3..fa910dc7fe3 100755 --- a/tests/functional/events/33-task-event-job-logs-retrieve-3.t +++ b/tests/functional/events/33-task-event-job-logs-retrieve-3.t @@ -16,17 +16,24 @@ # along with this program. If not, see . #------------------------------------------------------------------------------- # Test remote job logs retrieval OK with only "job.out" on a succeeded task. -export REQUIRE_PLATFORM='loc:remote' +export REQUIRE_PLATFORM='loc:remote fs:indep comms:tcp' . "$(dirname "$0")/test_header" set_test_number 5 +create_test_global_config "" " +[platforms] + [[blackbriar]] + hosts = ${CYLC_TEST_HOST} + install target = ${CYLC_TEST_INSTALL_TARGET} + retrieve job logs = True + retrieve job logs retry delays = 2*PT5S +" + install_workflow "${TEST_NAME_BASE}" "${TEST_NAME_BASE}" -run_ok "${TEST_NAME_BASE}-validate" \ - cylc validate -s "PLATFORM='${CYLC_TEST_PLATFORM}'" "${WORKFLOW_NAME}" +run_ok "${TEST_NAME_BASE}-validate" cylc validate "${WORKFLOW_NAME}" workflow_run_fail "${TEST_NAME_BASE}-run" \ - cylc play --reference-test --debug --no-detach \ - -s "PLATFORM='${CYLC_TEST_PLATFORM}'" "${WORKFLOW_NAME}" + cylc play --reference-test -vv --no-detach "${WORKFLOW_NAME}" sed "/'job-logs-retrieve'/!d" \ "${WORKFLOW_RUN_DIR}/log/job/1/t1/01/job-activity.log" \ diff --git a/tests/functional/events/33-task-event-job-logs-retrieve-3/flow.cylc b/tests/functional/events/33-task-event-job-logs-retrieve-3/flow.cylc index 6853299d9a2..1853454d218 100644 --- a/tests/functional/events/33-task-event-job-logs-retrieve-3/flow.cylc +++ b/tests/functional/events/33-task-event-job-logs-retrieve-3/flow.cylc @@ -10,13 +10,10 @@ [scheduling] [[graph]] - R1=t1 + R1 = t1 [runtime] [[t1]] - script=false - err-script=rm -f "${CYLC_TASK_LOG_ROOT}.err" - platform = {{ PLATFORM }} - [[[remote]]] - retrieve job logs = True - retrieve job logs retry delays = 2*PT5S + script = false + err-script = rm -f "${CYLC_TASK_LOG_ROOT}.err" + platform = blackbriar diff --git a/tests/functional/intelligent-host-selection/02-badhosts.t b/tests/functional/intelligent-host-selection/02-badhosts.t index eb39ab4bff5..0689866d22f 100644 --- a/tests/functional/intelligent-host-selection/02-badhosts.t +++ b/tests/functional/intelligent-host-selection/02-badhosts.t @@ -33,6 +33,7 @@ create_test_global_config "" " job runner = my_background hosts = e9755ca30f5, 3c0b4799402 install target = ${CYLC_TEST_INSTALL_TARGET} + retrieve job logs = True [[[selection]]] method = definition order diff --git a/tests/functional/intelligent-host-selection/02-badhosts/flow.cylc b/tests/functional/intelligent-host-selection/02-badhosts/flow.cylc index 86a86d60539..9cfa4a52def 100644 --- a/tests/functional/intelligent-host-selection/02-badhosts/flow.cylc +++ b/tests/functional/intelligent-host-selection/02-badhosts/flow.cylc @@ -23,8 +23,6 @@ Tasks [runtime] [[root]] script = true - [[[remote]]] - retrieve job logs = True [[badhosttask]] platform = badhostplatform diff --git a/tests/unit/test_platforms.py b/tests/unit/test_platforms.py index 322f41dbacc..8bbe19ebbd4 100644 --- a/tests/unit/test_platforms.py +++ b/tests/unit/test_platforms.py @@ -25,7 +25,7 @@ get_platform, get_platform_deprecated_settings, get_random_platform_for_install_target, is_platform_definition_subshell, - platform_from_name, platform_from_job_info, + platform_from_name, platform_name_from_job_info, get_install_target_from_platform, get_install_target_to_platforms_map, generic_items_match, @@ -208,7 +208,7 @@ def test_similar_but_not_exact_match(): # ---------------------------------------------------------------------------- -# Tests of platform_from_job_info +# Tests of platform_name_from_job_info # ---------------------------------------------------------------------------- # Basic tests that we can select sensible platforms @pytest.mark.parametrize( @@ -225,7 +225,7 @@ def test_similar_but_not_exact_match(): # returns to default, i.e. localhost. ( {'batch system': 'background'}, - {'retrieve job logs retry delays': 'None'}, + {'retrieve job logs retry delays': None}, 'localhost' ), # Check that when the user asks for batch system = slurm alone @@ -269,11 +269,11 @@ def test_similar_but_not_exact_match(): ), ] ) -def test_platform_from_job_info_basic(job, remote, returns): - assert platform_from_job_info(PLATFORMS, job, remote) == returns +def test_platform_name_from_job_info_basic(job, remote, returns): + assert platform_name_from_job_info(PLATFORMS, job, remote) == returns -def test_platform_from_job_info_ordered_dict_comparison(): +def test_platform_name_from_job_info_ordered_dict_comparison(): """Check that we are only comparing set items in OrderedDictWithDefaults. """ job = {'batch system': 'background', 'Made up key': 'Zaphod'} @@ -284,7 +284,7 @@ def test_platform_from_job_info_ordered_dict_comparison(): platform.defaults_['Made up key'] = {} platform.update(PLATFORMS['hpc1-bg']) platforms = {'hpc1-bg': platform, 'dobbie': PLATFORMS['sugar']} - assert platform_from_job_info(platforms, job, remote) == 'hpc1-bg' + assert platform_name_from_job_info(platforms, job, remote) == 'hpc1-bg' # Cases where the error ought to be raised because no matching platform should @@ -311,7 +311,7 @@ def test_platform_from_job_info_ordered_dict_comparison(): ) def test_reverse_PlatformLookupError(job, remote): with pytest.raises(PlatformLookupError): - platform_from_job_info(PLATFORMS, job, remote) + platform_name_from_job_info(PLATFORMS, job, remote) # An example of a global config with two Spice systems available @@ -335,7 +335,7 @@ def test_reverse_PlatformLookupError(job, remote): ) ] ) -def test_platform_from_job_info_two_spices( +def test_platform_name_from_job_info_two_spices( job, remote, returns ): platforms = { @@ -349,7 +349,7 @@ def test_platform_from_job_info_two_spices( }, } - assert platform_from_job_info(platforms, job, remote) == returns + assert platform_name_from_job_info(platforms, job, remote) == returns # An example of two platforms with the same hosts and job runner settings @@ -375,7 +375,7 @@ def test_platform_from_job_info_two_spices( ), ] ) -def test_platform_from_job_info_similar_platforms( +def test_platform_name_from_job_info_similar_platforms( job, remote, returns ): platforms = { @@ -396,7 +396,7 @@ def test_platform_from_job_info_similar_platforms( 'job runner': 'background' }, } - assert platform_from_job_info(platforms, job, remote) == returns + assert platform_name_from_job_info(platforms, job, remote) == returns # ----------------------------------------------------------------------------- @@ -555,9 +555,12 @@ def test_get_all_platforms_for_install_target(mock_glbl_cfg): @pytest.mark.parametrize( 'task_conf, expected', [ - ( + pytest.param( { - 'remote': {'host': 'cylcdevbox'}, + 'remote': { + 'host': 'cylcdevbox', + 'retrieve job logs': True + }, 'job': { 'batch system': 'pbs', 'batch submit command template': 'meow' @@ -565,11 +568,13 @@ def test_get_all_platforms_for_install_target(mock_glbl_cfg): }, [ '[runtime][task][job]batch submit command template = meow', + '[runtime][task][remote]retrieve job logs = True', '[runtime][task][remote]host = cylcdevbox', '[runtime][task][job]batch system = pbs' - ] + ], + id="All are deprecated settings" ), - ( + pytest.param( { 'remote': {'host': 'localhost'}, 'job': { @@ -577,12 +582,23 @@ def test_get_all_platforms_for_install_target(mock_glbl_cfg): 'batch submit command template': None } }, - ['[runtime][task][job]batch system = pbs'] + ['[runtime][task][job]batch system = pbs'], + id="Exclusions are excluded" + ), + pytest.param( + { + 'environment filter': { + 'include': ['frodo', 'sam'] + } + }, + [], + id="No deprecated settings" ) ] ) def test_get_platform_deprecated_settings( - task_conf: Dict[str, Any], expected: List[str]): + task_conf: Dict[str, Any], expected: List[str] +): output = get_platform_deprecated_settings(task_conf, task_name='task') assert set(output) == set(expected) @@ -621,14 +637,13 @@ def test_get_platform_from_OrderedDictWithDefaults(mock_glbl_cfg): ''' ) task_conf = OrderedDictWithDefaults() - task_conf.defaults_ = dict([ - ('job', dict([ + task_conf.defaults_ = OrderedDictWithDefaults([ + ('job', OrderedDictWithDefaults([ ('batch system', 'slurm') ])), - ('remote', dict([ + ('remote', OrderedDictWithDefaults([ ('host', 'foo') - ])) + ])), ]) result = get_platform(task_conf)['name'] assert result == 'skarloey' - diff --git a/tests/unit/test_platforms_get_platform.py b/tests/unit/test_platforms_get_platform.py index 2870fa2b641..d26af0575b2 100644 --- a/tests/unit/test_platforms_get_platform.py +++ b/tests/unit/test_platforms_get_platform.py @@ -16,9 +16,8 @@ # # Tests for the platform lookup module's get_platform method. -from typing import Dict, Optional +from typing import Callable, Dict, Optional import pytest -import random from cylc.flow.platforms import ( get_localhost_install_target, get_platform @@ -87,20 +86,62 @@ def test_get_platform_from_platform_name_str(mock_glbl_cfg, platform_re): assert platform['job runner'] == 'slurm' -def test_get_platform_cylc7_8_syntax_mix_fails(mock_glbl_cfg): +@pytest.mark.parametrize( + 'task_conf, err_expected', + [ + ( + { + 'platform': 'localhost', + 'remote': { + 'host': 'localhost' + } + }, + True + ), + ( + { + 'platform': 'gondor', + 'remote': { + 'retrieve job logs': False + } + }, + True + ), + ( + { + 'platform': 'gondor', + 'remote': { + 'host': None + } + }, + False + ), + ] +) +def test_get_platform_cylc7_8_syntax_mix_fails( + task_conf: dict, err_expected: bool, mock_glbl_cfg: Callable +): """If a task with a mix of Cylc7 and 8 syntax is passed to get_platform this should return an error. """ - task_conf = { - 'platform': 'localhost', - 'remote': { - 'host': 'localhost' - } - } - with pytest.raises( - PlatformLookupError, - match=r'A mixture of Cylc 7 \(host\) and Cylc 8 \(platform\).*' - ): + mock_glbl_cfg( + 'cylc.flow.platforms.glbl_cfg', + ''' + [platforms] + [[gondor]] + hosts = denethor + ''' + ) + if err_expected: + with pytest.raises( + PlatformLookupError, + match=( + r"Task .* has the following deprecated '\[runtime\]' " + r"setting\(s\) which cannot be used with 'platform.*" + ) + ): + get_platform(task_conf) + else: get_platform(task_conf) @@ -167,7 +208,7 @@ def test_get_platform_from_config_with_platform_name(mock_glbl_cfg): ) ] ) -def test_get_platform_using_platform_from_job_info( +def test_get_platform_using_platform_name_from_job_info( mock_glbl_cfg, task_conf, expected_platform_name ): """Calculate platform from Cylc 7 config: n.b. If this fails we don't