Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cylc VR: Validate should not change options #6316

Merged
merged 4 commits into from
Oct 8, 2024
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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.
wxtim marked this conversation as resolved.
Show resolved Hide resolved
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
14 changes: 7 additions & 7 deletions tests/functional/data-store/00-prune-optional-break.t
Original file line number Diff line number Diff line change
Expand Up @@ -27,25 +27,25 @@ init_workflow "${TEST_NAME_BASE}" << __FLOW__
final cycle point = 1
[[graph]]
P1 = """
a? => b? => c?
d => e
"""
a => b? => c?
a => d => e
"""
[runtime]
[[a,c,e]]
script = true
[[b]]
script = false
[[d]]
script = """
cylc workflow-state \${CYLC_WORKFLOW_ID}//1/b:failed --interval=2
cylc pause \$CYLC_WORKFLOW_ID
"""
cylc workflow-state \${CYLC_WORKFLOW_ID}//1/b:failed --interval=2 --max-polls=20 -v
cylc pause \$CYLC_WORKFLOW_ID
"""
__FLOW__

# run workflow
run_ok "${TEST_NAME_BASE}-run" cylc play "${WORKFLOW_NAME}"

cylc workflow-state "${WORKFLOW_NAME}/1/d:succeeded" --interval=2 --max-polls=60
cylc workflow-state "${WORKFLOW_NAME}//1/d:succeeded" --interval=2 --max-polls=60 -v

# query workflow
TEST_NAME="${TEST_NAME_BASE}-prune-optional-break"
Expand Down
3 changes: 2 additions & 1 deletion tests/functional/flow-triggers/10-specific-flow.t
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
#!/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
Expand All @@ -15,6 +15,7 @@
# You should have received a copy of the GNU General Public License
# along with this program. If not, see <http://www.gnu.org/licenses/>.
#-------------------------------------------------------------------------------
# Test targeting a specific flow, with trigger --wait.

. "$(dirname "$0")/test_header"
set_test_number 2
Expand Down
8 changes: 6 additions & 2 deletions tests/functional/flow-triggers/10-specific-flow/flow.cylc
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,10 @@
[[trigger-happy]]
script = """
cylc trigger --flow=2 --wait ${CYLC_WORKFLOW_ID}//1/f
cylc__job__poll_grep_workflow_log "1/d/01:submitted.*running"
cylc trigger --flow=2 ${CYLC_WORKFLOW_ID}//1/b
"""
[[d]]
script = """
if [[ "$CYLC_TASK_SUBMIT_NUMBER" == "1" ]]; then
cylc trigger --flow=2 ${CYLC_WORKFLOW_ID}//1/b
fi
"""
2 changes: 1 addition & 1 deletion tests/functional/reload/26-stalled.t
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ init_workflow "${TEST_NAME_BASE}" <<'__FLOW__'
[scheduler]
[[events]]
stall handlers = cylc reload %(workflow)s
stall timeout = PT10S
stall timeout = PT30S
abort on stall timeout = True
# Prevent infinite loop if the bug resurfaces
workflow timeout = PT3M
Expand Down
3 changes: 2 additions & 1 deletion tests/functional/triggering/08-fam-finish-any.t
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
#!/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
Expand All @@ -17,6 +17,7 @@
#-------------------------------------------------------------------------------
# Test correct expansion of 'FAM:finish-any'
. "$(dirname "$0")/test_header"
skip_macos_gh_actions
set_test_number 2
reftest
exit
14 changes: 7 additions & 7 deletions tests/unit/test_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -267,7 +267,7 @@ def test_family_inheritance_and_quotes(


@pytest.mark.parametrize(
('cycling_type', 'scheduling_cfg', 'expected_icp', 'expected_opt_icp',
('cycling_type', 'scheduling_cfg', 'expected_icp', 'expected_eval_icp',
'expected_err'),
[
pytest.param(
Expand Down Expand Up @@ -356,7 +356,7 @@ def test_process_icp(
cycling_type: str,
scheduling_cfg: Dict[str, Any],
expected_icp: Optional[str],
expected_opt_icp: Optional[str],
expected_eval_icp: Optional[str],
expected_err: Optional[Tuple[Type[Exception], str]],
monkeypatch: pytest.MonkeyPatch, set_cycling_type: Fixture
) -> None:
Expand All @@ -368,7 +368,7 @@ def test_process_icp(
cycling_type: Workflow cycling type.
scheduling_cfg: 'scheduling' section of workflow config.
expected_icp: The expected icp value that gets set.
expected_opt_icp: The expected value of options.icp that gets set
expected_eval_icp: The expected value of options.icp that gets set
(this gets stored in the workflow DB).
expected_err: Exception class expected to be raised plus the message.
"""
Expand Down Expand Up @@ -396,10 +396,10 @@ 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
if opt_icp is not None:
opt_icp = str(loader.get_point(opt_icp).standardise())
assert opt_icp == expected_opt_icp
eval_icp = mocked_config.evaluated_icp
if eval_icp is not None:
eval_icp = str(loader.get_point(eval_icp).standardise())
assert eval_icp == expected_eval_icp


@pytest.mark.parametrize(
Expand Down
Loading