Skip to content

Commit

Permalink
Cylc VR: Don't save icp to options object to
Browse files Browse the repository at this point in the history
prevent changes made to the options by validate affecting
subsequent steps.

Response to review
- Variable name suggestions.
- Test speedup.
  • Loading branch information
wxtim committed Aug 28, 2024
1 parent 347921f commit 0b53414
Show file tree
Hide file tree
Showing 6 changed files with 60 additions and 5 deletions.
1 change: 1 addition & 0 deletions changes.d/6316.fix.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Prevent validate step of VR changing CLI options.
7 changes: 4 additions & 3 deletions cylc/flow/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
"""
Expand All @@ -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)

Expand Down
7 changes: 6 additions & 1 deletion cylc/flow/workflow_db_mgr.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
39 changes: 39 additions & 0 deletions tests/functional/cylc-combination-scripts/09-vr-icp-now.t
Original file line number Diff line number Diff line change
@@ -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 <http://www.gnu.org/licenses/>.

#------------------------------------------------------------------------------
# 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
Original file line number Diff line number Diff line change
@@ -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
2 changes: 1 addition & 1 deletion tests/unit/test_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit 0b53414

Please sign in to comment.