From 9b4bac38ffff958eaa437f5bcede432d71506a97 Mon Sep 17 00:00:00 2001 From: Oliver Sanders Date: Wed, 16 Oct 2024 12:28:18 +0100 Subject: [PATCH] broadcast: reject truncated cycle points (#6414) --- changes.d/6414.fix.md | 1 + cylc/flow/cycling/__init__.py | 11 +++++-- cylc/flow/cycling/integer.py | 2 +- cylc/flow/cycling/iso8601.py | 9 +++++- cylc/flow/cycling/loader.py | 4 +-- tests/integration/scripts/test_broadcast.py | 35 +++++++++++++++++++++ 6 files changed, 56 insertions(+), 6 deletions(-) create mode 100644 changes.d/6414.fix.md diff --git a/changes.d/6414.fix.md b/changes.d/6414.fix.md new file mode 100644 index 00000000000..cecc178dd07 --- /dev/null +++ b/changes.d/6414.fix.md @@ -0,0 +1 @@ +Broadcast will now reject truncated cycle points to aviod runtime errors. diff --git a/cylc/flow/cycling/__init__.py b/cylc/flow/cycling/__init__.py index c47d242b5c5..7c644263189 100644 --- a/cylc/flow/cycling/__init__.py +++ b/cylc/flow/cycling/__init__.py @@ -91,8 +91,15 @@ def _cmp(self, other) -> int: """Compare self to other point, returning a 'cmp'-like result.""" pass - def standardise(self) -> 'PointBase': - """Format self.value into a standard representation and check it.""" + def standardise(self, allow_truncated: bool = True) -> 'PointBase': + """Format self.value into a standard representation and check it. + + Args: + allow_truncated: + If True, then truncated points (i.e. any point with context + missing off the front) will be tollerated, if False, truncated + points will cause an exception to be raised. + """ return self @abstractmethod diff --git a/cylc/flow/cycling/integer.py b/cylc/flow/cycling/integer.py index 749c651fc08..c963804a7d5 100644 --- a/cylc/flow/cycling/integer.py +++ b/cylc/flow/cycling/integer.py @@ -145,7 +145,7 @@ def sub(self, other): return IntegerInterval.from_integer(int(self) - int(other)) return IntegerPoint(int(self) - int(other)) - def standardise(self): + def standardise(self, allow_truncated=True): """Format self.value into a standard representation and check it.""" try: self.value = str(int(self)) diff --git a/cylc/flow/cycling/iso8601.py b/cylc/flow/cycling/iso8601.py index a66ce3f5ba0..3d7cf42bc3e 100644 --- a/cylc/flow/cycling/iso8601.py +++ b/cylc/flow/cycling/iso8601.py @@ -92,9 +92,16 @@ def add(self, other): self.value, other.value, CALENDAR.mode )) - def standardise(self): + def standardise(self, allow_truncated=True): """Reformat self.value into a standard representation.""" try: + point = point_parse(self.value) + if not allow_truncated and point.truncated: + raise PointParsingError( + type(self), + self.value, + 'Truncated ISO8601 dates are not permitted', + ) self.value = str(point_parse(self.value)) except IsodatetimeError as exc: if self.value.startswith("+") or self.value.startswith("-"): diff --git a/cylc/flow/cycling/loader.py b/cylc/flow/cycling/loader.py index 71c175eb97a..11b86f0819c 100644 --- a/cylc/flow/cycling/loader.py +++ b/cylc/flow/cycling/loader.py @@ -158,13 +158,13 @@ def standardise_point_string( def standardise_point_string( - point_string: Optional[str], cycling_type: Optional[str] = None + point_string: Optional[str], cycling_type: Optional[str] = None, ) -> Optional[str]: """Return a standardised version of point_string.""" if point_string is None: return None point = get_point(point_string, cycling_type=cycling_type) if point is not None: - point.standardise() + point.standardise(allow_truncated=False) point_string = str(point) return point_string diff --git a/tests/integration/scripts/test_broadcast.py b/tests/integration/scripts/test_broadcast.py index 163d48e552e..6d3b4cd0db8 100644 --- a/tests/integration/scripts/test_broadcast.py +++ b/tests/integration/scripts/test_broadcast.py @@ -134,3 +134,38 @@ async def test_broadcast_multi_namespace( ('*', 'VOWELS', 'execution time limit', 'PT5S'), ('*', 'root', 'execution time limit', 'PT5S'), ] + + +async def test_broadcast_truncated_datetime(flow, scheduler, start, capsys): + """It should reject truncated datetime cycle points. + + See https://github.com/cylc/cylc-flow/issues/6407 + """ + id_ = flow({ + 'scheduling': { + 'initial cycle point': '2000', + 'graph': { + 'R1': 'foo', + }, + } + }) + schd = scheduler(id_) + async with start(schd): + # attempt an invalid broadcast + rets = await _main( + BroadcastOptions( + settings=['[environment]FOO=bar'], + point_strings=['050101T0000Z'], # <== truncated + ), + schd.workflow, + ) + + # the broadcast should fail + assert list(rets.values()) == [False] + + # an error should be recorded + _out, err = capsys.readouterr() + assert ( + 'Rejected broadcast:' + ' settings are not compatible with the workflow' + ) in err