From 0964342c145eb60887c89b143b4971bded291d57 Mon Sep 17 00:00:00 2001 From: Tim Pillinger <26465611+wxtim@users.noreply.github.com> Date: Fri, 10 May 2024 13:01:47 +0100 Subject: [PATCH 1/2] check whether workflow should be in back compatibility mode before opening the flow-processed.cylc file, and override the compat mode in the workflow config if required. --- cylc/rose/platform_utils.py | 17 ++++++++++++++++- tests/unit/test_platform_utils.py | 28 ++++++++++++++++++++++++---- 2 files changed, 40 insertions(+), 5 deletions(-) diff --git a/cylc/rose/platform_utils.py b/cylc/rose/platform_utils.py index 74435a5b..476e3cca 100644 --- a/cylc/rose/platform_utils.py +++ b/cylc/rose/platform_utils.py @@ -19,6 +19,7 @@ """Interfaces for Cylc Platforms for use by rose apps.""" from optparse import Values +from pathlib import Path import sqlite3 import subprocess from time import sleep @@ -57,7 +58,12 @@ def get_platform_from_task_def(flow: str, task: str) -> Dict[str, Any]: workflow_id, WorkflowFiles.FLOW_FILE_PROCESSED, ) - config = WorkflowConfig(flow, flow_file, Values()) + + config = WorkflowConfig( + flow, + flow_file, Values(), + force_compat_mode=force_compat_mode(flow_file) + ) # Get entire task spec to allow Cylc 7 platform from host guessing. task_spec = config.pcfg.get(['runtime', task]) # check for subshell and evaluate @@ -76,6 +82,15 @@ def get_platform_from_task_def(flow: str, task: str) -> Dict[str, Any]: return platform +def force_compat_mode(flow_file): + """Check whether this is a Cylc 7 Back compatibility mode workflow: + https://github.com/cylc/cylc-rose/issues/319 + """ + return ( + Path(flow_file).parent.parent.parent / WorkflowFiles.SUITE_RC + ).exists() + + def eval_subshell(platform): """Evaluates platforms/hosts defined as subshell""" match = HOST_REC_COMMAND.match(platform) diff --git a/tests/unit/test_platform_utils.py b/tests/unit/test_platform_utils.py index b60fe7d4..de5955ca 100644 --- a/tests/unit/test_platform_utils.py +++ b/tests/unit/test_platform_utils.py @@ -24,6 +24,12 @@ import sqlite3 from uuid import uuid4 +from cylc.rose.platform_utils import ( + force_compat_mode, + get_platform_from_task_def, + get_platforms_from_task_jobs, +) + from cylc.flow import __version__ as cylc_version from cylc.flow.cfgspec.globalcfg import SPEC from cylc.flow.parsec.config import ParsecConfig @@ -31,10 +37,6 @@ from cylc.flow.workflow_db_mgr import CylcWorkflowDAO import pytest -from cylc.rose.platform_utils import ( - get_platform_from_task_def, - get_platforms_from_task_jobs, -) MOCK_GLBL_CFG = ( 'cylc.flow.platforms.glbl_cfg', @@ -211,6 +213,24 @@ def test_get_platform_from_task_def_subshell( assert platform['name'] == expected +@pytest.mark.parametrize( + 'create, expect', + ( + (['suite.rc', 'log/conf/flow-processed.cylc'], True), + (['suite.rc', 'foo/bar/any-old.file'], True), + (['flow.cylc', 'log/conf/flow-processed.cylc'], False), + (['flow.cylc', 'where/flow-processed.cylc'], False), + ) +) +def test_force_compat_mode(tmp_path, create, expect): + """It checks whether there is a suite.rc two directories up.""" + for file in create: + file = tmp_path / file + file.parent.mkdir(parents=True, exist_ok=True) + file.touch() + assert force_compat_mode(file) == expect + + @pytest.mark.parametrize( 'task, cycle, expect', [ From bea2f907fe137ca02529597fb302c08fa1db7b81 Mon Sep 17 00:00:00 2001 From: Tim Pillinger <26465611+wxtim@users.noreply.github.com> Date: Mon, 13 May 2024 08:09:24 +0100 Subject: [PATCH 2/2] Review suggestion: Make function work with cylc run dir. Thanks @oliver-sanders --- cylc/rose/platform_utils.py | 17 ++++++++++------- tests/unit/test_platform_utils.py | 6 +++--- 2 files changed, 13 insertions(+), 10 deletions(-) diff --git a/cylc/rose/platform_utils.py b/cylc/rose/platform_utils.py index 476e3cca..ea30c3f4 100644 --- a/cylc/rose/platform_utils.py +++ b/cylc/rose/platform_utils.py @@ -23,12 +23,13 @@ import sqlite3 import subprocess from time import sleep -from typing import Any, Dict +from typing import Any, Dict, Union from cylc.flow.config import WorkflowConfig from cylc.flow.id_cli import parse_id from cylc.flow.pathutil import ( get_workflow_run_config_log_dir, + get_workflow_run_dir, get_workflow_run_pub_db_path, ) from cylc.flow.workflow_files import WorkflowFiles @@ -62,7 +63,7 @@ def get_platform_from_task_def(flow: str, task: str) -> Dict[str, Any]: config = WorkflowConfig( flow, flow_file, Values(), - force_compat_mode=force_compat_mode(flow_file) + force_compat_mode=get_compat_mode(get_workflow_run_dir(workflow_id)) ) # Get entire task spec to allow Cylc 7 platform from host guessing. task_spec = config.pcfg.get(['runtime', task]) @@ -82,13 +83,15 @@ def get_platform_from_task_def(flow: str, task: str) -> Dict[str, Any]: return platform -def force_compat_mode(flow_file): +def get_compat_mode(run_dir: Union[str, Path]) -> bool: """Check whether this is a Cylc 7 Back compatibility mode workflow: - https://github.com/cylc/cylc-rose/issues/319 + + See https://github.com/cylc/cylc-rose/issues/319 + + Args: + run_dir: Cylc workflow run directory. """ - return ( - Path(flow_file).parent.parent.parent / WorkflowFiles.SUITE_RC - ).exists() + return (Path(run_dir) / WorkflowFiles.SUITE_RC).exists() def eval_subshell(platform): diff --git a/tests/unit/test_platform_utils.py b/tests/unit/test_platform_utils.py index de5955ca..e615982e 100644 --- a/tests/unit/test_platform_utils.py +++ b/tests/unit/test_platform_utils.py @@ -25,7 +25,7 @@ from uuid import uuid4 from cylc.rose.platform_utils import ( - force_compat_mode, + get_compat_mode, get_platform_from_task_def, get_platforms_from_task_jobs, ) @@ -222,13 +222,13 @@ def test_get_platform_from_task_def_subshell( (['flow.cylc', 'where/flow-processed.cylc'], False), ) ) -def test_force_compat_mode(tmp_path, create, expect): +def test_get_compat_mode(tmp_path, create, expect): """It checks whether there is a suite.rc two directories up.""" for file in create: file = tmp_path / file file.parent.mkdir(parents=True, exist_ok=True) file.touch() - assert force_compat_mode(file) == expect + assert get_compat_mode(tmp_path) == expect @pytest.mark.parametrize(