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

broadcast: reject truncated cycle points #6414

Merged
merged 2 commits into from
Oct 16, 2024
Merged
Show file tree
Hide file tree
Changes from all 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/6414.fix.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Broadcast will now reject truncated cycle points to aviod runtime errors.
11 changes: 9 additions & 2 deletions cylc/flow/cycling/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion cylc/flow/cycling/integer.py
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand Down
9 changes: 8 additions & 1 deletion cylc/flow/cycling/iso8601.py
Original file line number Diff line number Diff line change
Expand Up @@ -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("-"):
Expand Down
4 changes: 2 additions & 2 deletions cylc/flow/cycling/loader.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
35 changes: 35 additions & 0 deletions tests/integration/scripts/test_broadcast.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Loading