From 0b53414e99031a4e554a29608f49f9e5eae077e1 Mon Sep 17 00:00:00 2001 From: Tim Pillinger Date: Thu, 22 Aug 2024 10:45:05 +0100 Subject: [PATCH] Cylc VR: Don't save icp to options object to prevent changes made to the options by validate affecting subsequent steps. Response to review - Variable name suggestions. - Test speedup. --- changes.d/6316.fix.md | 1 + cylc/flow/config.py | 7 ++-- cylc/flow/workflow_db_mgr.py | 7 +++- .../cylc-combination-scripts/09-vr-icp-now.t | 39 +++++++++++++++++++ .../09-vr-icp-now/flow.cylc | 9 +++++ tests/unit/test_config.py | 2 +- 6 files changed, 60 insertions(+), 5 deletions(-) create mode 100644 changes.d/6316.fix.md create mode 100644 tests/functional/cylc-combination-scripts/09-vr-icp-now.t create mode 100644 tests/functional/cylc-combination-scripts/09-vr-icp-now/flow.cylc diff --git a/changes.d/6316.fix.md b/changes.d/6316.fix.md new file mode 100644 index 00000000000..956b0c06066 --- /dev/null +++ b/changes.d/6316.fix.md @@ -0,0 +1 @@ +Prevent validate step of VR changing CLI options. diff --git a/cylc/flow/config.py b/cylc/flow/config.py index c5e82d74d97..9cb19ac7d2c 100644 --- a/cylc/flow/config.py +++ b/cylc/flow/config.py @@ -690,7 +690,7 @@ def process_initial_cycle_point(self) -> None: Sets: self.initial_point self.cfg['scheduling']['initial cycle point'] - self.options.icp + self.evaluated_icp Raises: WorkflowConfigError - if it fails to validate """ @@ -710,10 +710,11 @@ def process_initial_cycle_point(self) -> None: icp = ingest_time(orig_icp, get_current_time_string()) except IsodatetimeError as exc: raise WorkflowConfigError(str(exc)) - if orig_icp != icp: + self.evaluated_icp = None + if icp != orig_icp: # now/next()/previous() was used, need to store # evaluated point in DB - self.options.icp = icp + self.evaluated_icp = icp self.initial_point = get_point(icp).standardise() self.cfg['scheduling']['initial cycle point'] = str(self.initial_point) diff --git a/cylc/flow/workflow_db_mgr.py b/cylc/flow/workflow_db_mgr.py index 8e57d19292b..9b5b0c3a8f6 100644 --- a/cylc/flow/workflow_db_mgr.py +++ b/cylc/flow/workflow_db_mgr.py @@ -327,8 +327,13 @@ def put_workflow_params(self, schd: 'Scheduler') -> None: {"key": self.KEY_STOP_CLOCK_TIME, "value": schd.stop_clock_time}, {"key": self.KEY_STOP_TASK, "value": schd.stop_task}, ]) + + # Retrieve raw initial cycle point stored on the config. + value = getattr(schd.config, 'evaluated_icp', None) + value = None if value == 'reload' else value + self.put_workflow_params_1(self.KEY_INITIAL_CYCLE_POINT, value) + for key in ( - self.KEY_INITIAL_CYCLE_POINT, self.KEY_FINAL_CYCLE_POINT, self.KEY_START_CYCLE_POINT, self.KEY_STOP_CYCLE_POINT diff --git a/tests/functional/cylc-combination-scripts/09-vr-icp-now.t b/tests/functional/cylc-combination-scripts/09-vr-icp-now.t new file mode 100644 index 00000000000..932e735fff1 --- /dev/null +++ b/tests/functional/cylc-combination-scripts/09-vr-icp-now.t @@ -0,0 +1,39 @@ +#!/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 validate step of Cylc VR cannot change the options object. +# See https://github.com/cylc/cylc-flow/issues/6262 + +. "$(dirname "$0")/test_header" +set_test_number 2 + +WORKFLOW_ID=$(workflow_id) + +cp -r "${TEST_SOURCE_DIR}/${TEST_NAME_BASE}/flow.cylc" . + +run_ok "${TEST_NAME_BASE}-vip" \ + cylc vip . \ + --workflow-name "${WORKFLOW_ID}" \ + --no-detach \ + --no-run-name + +echo "# Some Comment" >> flow.cylc + +run_ok "${TEST_NAME_BASE}-vr" \ + cylc vr "${WORKFLOW_ID}" \ + --stop-cycle-point 2020-01-01T00:02Z diff --git a/tests/functional/cylc-combination-scripts/09-vr-icp-now/flow.cylc b/tests/functional/cylc-combination-scripts/09-vr-icp-now/flow.cylc new file mode 100644 index 00000000000..e9f6284769e --- /dev/null +++ b/tests/functional/cylc-combination-scripts/09-vr-icp-now/flow.cylc @@ -0,0 +1,9 @@ +[scheduling] + initial cycle point = 2020 + stop after cycle point = 2020-01-01T00:01Z + [[graph]] + PT1M = foo +[runtime] + [[foo]] + [[[simulation]]] + default run length = PT0S diff --git a/tests/unit/test_config.py b/tests/unit/test_config.py index 9cdcee89003..f7bd7292f48 100644 --- a/tests/unit/test_config.py +++ b/tests/unit/test_config.py @@ -396,7 +396,7 @@ def test_process_icp( assert mocked_config.cfg[ 'scheduling']['initial cycle point'] == expected_icp assert str(mocked_config.initial_point) == expected_icp - opt_icp = mocked_config.options.icp + opt_icp = mocked_config.evaluated_icp if opt_icp is not None: opt_icp = str(loader.get_point(opt_icp).standardise()) assert opt_icp == expected_opt_icp