Skip to content

Commit 91d3662

Browse files
wxtimwxtim
committed
Improve and refactor checking of initial, start, stop and final cycle points
1 parent c94a944 commit 91d3662

File tree

5 files changed

+158
-16
lines changed

5 files changed

+158
-16
lines changed

‎changes.d/6874.fix.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Improved warnings for suspect start and stop cycle points.

‎cylc/flow/config.py‎

Lines changed: 31 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -476,6 +476,10 @@ def __init__(
476476
self.process_start_cycle_point()
477477
self.process_final_cycle_point()
478478
self.process_stop_cycle_point()
479+
if self.start_point:
480+
self.cycle_point_warning('start', 'before', 'initial')
481+
self.cycle_point_warning('start', 'after', 'final')
482+
self.cycle_point_warning('stop', 'before', 'start')
479483

480484
# Parse special task cycle point offsets, and replace family names.
481485
LOG.debug("Parsing [special tasks]")
@@ -810,6 +814,7 @@ def process_start_cycle_point(self) -> None:
810814
if self.options.startcp == 'now':
811815
self.options.startcp = get_current_time_string()
812816
self.start_point = get_point(self.options.startcp).standardise()
817+
813818
elif starttask:
814819
# Start from designated task(s).
815820
# Select the earliest start point for use in pre-initial ignore.
@@ -828,6 +833,24 @@ def process_start_cycle_point(self) -> None:
828833
# Start from the initial point.
829834
self.start_point = self.initial_point
830835

836+
def cycle_point_warning(self, label1, order, label2):
837+
"""Centralized logic for warning that start, stop, initial and
838+
final cycle points are not sensibly ordered.
839+
"""
840+
point1 = getattr(self, label1 + '_point')
841+
point2 = getattr(self, label2 + '_point')
842+
if (
843+
order == 'before' and point1 < point2
844+
or order == 'after' and point1 > point2
845+
):
846+
msg = (
847+
f"{label1} cycle point '{point1}' will have no effect as"
848+
f" it is {order} the {label2} cycle point '{point2}'."
849+
)
850+
LOG.warning(msg)
851+
return True
852+
return False
853+
831854
def process_final_cycle_point(self) -> None:
832855
"""Validate and set the final cycle point from flow.cylc or options.
833856
@@ -908,15 +931,15 @@ def process_stop_cycle_point(self) -> None:
908931
self.initial_point,
909932
).standardise()
910933
if (
911-
self.final_point is not None
912-
and self.stop_point is not None
913-
and self.stop_point > self.final_point
914-
):
915-
LOG.warning(
916-
f"Stop cycle point '{self.stop_point}' will have no "
917-
"effect as it is after the final cycle "
918-
f"point '{self.final_point}'."
934+
self.stop_point is not None
935+
and (
936+
(
937+
self.final_point is not None
938+
and self.cycle_point_warning('stop', 'after', 'final')
939+
)
940+
or self.cycle_point_warning('stop', 'before', 'initial')
919941
)
942+
):
920943
self.stop_point = None
921944
stopcp_str = str(self.stop_point) if self.stop_point else None
922945
self.cfg['scheduling']['stop after cycle point'] = stopcp_str

‎tests/functional/cylc-play/06-warnif-scp-after-fcp.t‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -46,10 +46,10 @@ for SCP in 1 2 3; do
4646
--no-detach --stopcp="${SCP}"
4747

4848
if [[ "${SCP}" -lt 3 ]]; then
49-
grep_ok "Stop cycle point '.*'.*after.*final cycle point '.*'" \
49+
grep_ok "stop cycle point '.*'.*after.*final cycle point '.*'" \
5050
"${RUN_DIR}/${WORKFLOW_NAME}/log/scheduler/log" "-v"
5151
else
52-
grep_ok "Stop cycle point '.*'.*after.*final cycle point '.*'" \
52+
grep_ok "stop cycle point '.*'.*after.*final cycle point '.*'" \
5353
"${RUN_DIR}/${WORKFLOW_NAME}/log/scheduler/log"
5454
fi
5555
done

‎tests/integration/test_config.py‎

Lines changed: 116 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -739,3 +739,119 @@ async def test_task_event_bad_custom_template(
739739
with pytest.raises(WorkflowConfigError, match=exception):
740740
async with start(schd):
741741
pass
742+
743+
744+
@pytest.mark.parametrize(
745+
'a, b, c, d, validation_fail, err',
746+
(
747+
('initial', 'start', 'stop', 'final', True, False),
748+
(
749+
'initial', 'start', 'final', 'stop', True,
750+
"stop cycle point '20030101T0000Z' will have no effect as it"
751+
" is after the final cycle point '20020101T0000Z'."
752+
),
753+
(
754+
'initial', 'stop', 'start', 'final', False,
755+
"stop cycle point '20010101T0000Z' will have no effect as it"
756+
" is before the start cycle point '20020101T0000Z'."
757+
),
758+
(
759+
'initial', 'stop', 'final', 'start', False,
760+
"start cycle point '20030101T0000Z' will have no effect as it"
761+
" is after the final cycle point '20020101T0000Z'."
762+
),
763+
(
764+
'initial', 'final', 'start', 'stop', True,
765+
"stop cycle point '20030101T0000Z' will have no effect as it"
766+
" is after the final cycle point '20010101T0000Z'."
767+
),
768+
(
769+
'initial', 'final', 'stop', 'start', True,
770+
"stop cycle point '20020101T0000Z' will have no effect as it"
771+
" is after the final cycle point '20010101T0000Z'."
772+
),
773+
(
774+
'start', 'initial', 'stop', 'final', False,
775+
"start cycle point '20000101T0000Z' will have no effect as it"
776+
" is before the initial cycle point '20010101T0000Z'."
777+
),
778+
(
779+
'start', 'initial', 'final', 'stop', True,
780+
"stop cycle point '20030101T0000Z' will have no effect as it"
781+
" is after the final cycle point '20020101T0000Z'."
782+
),
783+
(
784+
'start', 'stop', 'initial', 'final', True,
785+
"stop cycle point '20010101T0000Z' will have no effect as it"
786+
" is before the initial cycle point '20020101T0000Z'."
787+
),
788+
('start', 'stop', 'final', 'initial', True, WorkflowConfigError),
789+
('start', 'final', 'initial', 'stop', True, WorkflowConfigError),
790+
('start', 'final', 'stop', 'initial', True, WorkflowConfigError),
791+
(
792+
'stop', 'initial', 'start', 'final', True,
793+
"stop cycle point '20000101T0000Z' will have no effect as it"
794+
" is before the initial cycle point '20010101T0000Z'."
795+
),
796+
(
797+
'stop', 'initial', 'final', 'start', True,
798+
"stop cycle point '20000101T0000Z' will have no effect as it"
799+
" is before the initial cycle point '20010101T0000Z'."
800+
),
801+
(
802+
'stop', 'start', 'initial', 'final', True,
803+
"stop cycle point '20000101T0000Z' will have no effect as it"
804+
" is before the initial cycle point '20020101T0000Z'."
805+
),
806+
('stop', 'start', 'final', 'initial', True, WorkflowConfigError),
807+
('stop', 'final', 'initial', 'start', True, WorkflowConfigError),
808+
('stop', 'final', 'start', 'initial', True, WorkflowConfigError),
809+
('final', 'initial', 'start', 'stop', True, WorkflowConfigError),
810+
('final', 'initial', 'stop', 'start', True, WorkflowConfigError),
811+
('final', 'start', 'initial', 'stop', True, WorkflowConfigError),
812+
('final', 'start', 'stop', 'initial', True, WorkflowConfigError),
813+
('final', 'stop', 'initial', 'start', True, WorkflowConfigError),
814+
('final', 'stop', 'start', 'initial', True, WorkflowConfigError),
815+
)
816+
)
817+
async def test_milestone_cycle_points(
818+
a,
819+
b,
820+
c,
821+
d,
822+
validation_fail,
823+
err,
824+
flow,
825+
validate,
826+
scheduler,
827+
start,
828+
log_filter,
829+
caplog,
830+
):
831+
"""Ensure that all combinations of initial, start, stop and final cycle
832+
point return sensible warnings or errors.
833+
"""
834+
order = dict(zip((a, b, c, d), [2000, 2001, 2002, 2003]))
835+
836+
wid = flow({
837+
'scheduling': {
838+
'initial cycle point': order['initial'],
839+
'stop after cycle point': order['stop'],
840+
'final cycle point': order['final'],
841+
'graph': {'P1Y': 'foo'}
842+
},
843+
})
844+
if validation_fail:
845+
if not err:
846+
validate(wid)
847+
elif isinstance(err, str):
848+
validate(wid)
849+
assert err in caplog.messages
850+
else:
851+
with pytest.raises(err):
852+
validate(wid)
853+
854+
else:
855+
schd = scheduler(wid, startcp=str(order['start']))
856+
async with start(schd):
857+
assert err in caplog.messages

‎tests/unit/test_config.py‎

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -14,10 +14,12 @@
1414
# You should have received a copy of the GNU General Public License
1515
# along with this program. If not, see <http://www.gnu.org/licenses/>.
1616

17+
from functools import partial
18+
import os
19+
from optparse import Values
20+
import pytest
1721
from contextlib import suppress
1822
import logging
19-
from optparse import Values
20-
import os
2123
from pathlib import Path
2224
from textwrap import dedent
2325
from types import SimpleNamespace
@@ -31,8 +33,6 @@
3133
Type,
3234
)
3335

34-
import pytest
35-
3636
from cylc.flow import (
3737
CYLC_LOG,
3838
flags,
@@ -709,7 +709,7 @@ def test_process_fcp(
709709
id="stopcp-beyond-fcp"
710710
),
711711
pytest.param(
712-
'+P12Y -P2Y', None, '2000', None, None,
712+
'+P12Y -P2Y', None, '1010', None, None,
713713
id="stopcp-relative-to-icp"
714714
),
715715
]
@@ -741,11 +741,13 @@ def test_process_stop_cycle_point(
741741
'stop after cycle point': cfg_stopcp
742742
}
743743
},
744-
initial_point=ISO8601Point('1990'),
744+
initial_point=ISO8601Point('1000'),
745745
final_point=fcp,
746746
stop_point=None,
747747
options=RunOptions(stopcp=options_stopcp),
748748
)
749+
mock_config.cycle_point_warning = partial(
750+
WorkflowConfig.cycle_point_warning, mock_config)
749751

750752
WorkflowConfig.process_stop_cycle_point(mock_config)
751753
assert str(mock_config.stop_point) == str(expected_value)

0 commit comments

Comments
 (0)