Skip to content

Commit

Permalink
Error if graph has a dependency offset exclusively on the RHS of a gr…
Browse files Browse the repository at this point in the history
…aph pair.
  • Loading branch information
wxtim committed Jan 16, 2024
1 parent b20a39c commit 7cd0bce
Show file tree
Hide file tree
Showing 3 changed files with 49 additions and 14 deletions.
1 change: 1 addition & 0 deletions changes.d/fix.5924.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Ensure that graphs with dependency offsets on the right hand side are flagged to users.
38 changes: 26 additions & 12 deletions cylc/flow/graph_parser.py
Original file line number Diff line number Diff line change
Expand Up @@ -462,8 +462,13 @@ def parse_graph(self, graph_string: str) -> None:
for i in range(0, len(chain) - 1):
pairs.add((chain[i], chain[i + 1]))

# Get a set of RH nodes which are not at the LH of another pair:
terminals = {
right for right in {right for _, right in pairs}
if right not in {left for left, _ in pairs}}

for pair in pairs:
self._proc_dep_pair(pair)
self._proc_dep_pair(pair, terminals)

@classmethod
def _report_invalid_lines(cls, lines: List[str]) -> None:
Expand Down Expand Up @@ -493,19 +498,23 @@ def _report_invalid_lines(cls, lines: List[str]) -> None:

def _proc_dep_pair(
self,
pair: Tuple[Optional[str], str]
pair: Tuple[Optional[str], str],
terminals: Set[str],
) -> None:
"""Process a single dependency pair 'left => right'.
'left' can be a logical expression of qualified node names.
'left' can be None, when triggering a left-side or lone node.
'left' can be "", if null task name in graph error (a => => b).
'right' can be one or more node names joined by AND.
'right' can't be None or "".
A node is an xtrigger, or a task or a family name.
A qualified name is NAME([CYCLE-POINT-OFFSET])(:QUALIFIER).
Trigger qualifiers, but not cycle offsets, are ignored on the right to
allow chaining.
Args:
pair:
'left' can be a logical expression of qualified node names.
'left' can be None, when triggering a left-side or lone node.
'left' can be "", if null task name in graph error (a => => b).
'right' can be one or more node names joined by AND.
'right' can't be None or "".
terminals:
Lits of nodes which are _only_ on the RH end of chains.
"""
left, right = pair
# Raise error for right-hand-side OR operators.
Expand All @@ -525,10 +534,15 @@ def _proc_dep_pair(
if right.count("(") != right.count(")"):
raise GraphParseError(mismatch_msg.format(right))

# Ignore cycle point offsets on the right side.
# (Note we can't ban this; all nodes get process as left and right.)
# Ignore/Error cycle point offsets on the right side:
# (Note we can only ban this for nodes at the end of chains)
if '[' in right:
return
if right in terminals:
raise GraphParseError(
'ERROR, illegal cycle point offset on the right:'
f' {left} => {right}')
else:
return

# Split right side on AND.
rights = right.split(self.__class__.OP_AND)
Expand Down
24 changes: 22 additions & 2 deletions tests/unit/test_graph_parser.py
Original file line number Diff line number Diff line change
Expand Up @@ -133,13 +133,33 @@ def test_graph_syntax_errors_2(seq, graph, expected_err):
"foo || bar => baz",
"The graph OR operator is '|'"
),
param(
# See https://github.com/cylc/cylc-flow/issues/5844
"foo => bar[1649]",
'illegal cycle point offset on the right',
id='no-cycle-point-RHS'
),
param(
"foo => bar[1649] => baz",
'!illegal cycle point offset on the right',
id='ignore-not-exclusively-RHS-cycle-point'
),
param(
"foo => bar[1649]\nbar[1649] => baz",
'!illegal cycle point offset on the right',
id='ignore-not-exclusively-RHS-cycle-point2'
),
]
)
def test_graph_syntax_errors(graph, expected_err):
"""Test various graph syntax errors."""
with pytest.raises(GraphParseError) as cm:
if expected_err[0] == '!':
# Assert method doesn't fail:
GraphParser().parse_graph(graph)
assert expected_err in str(cm.value)
else:
with pytest.raises(GraphParseError) as cm:
GraphParser().parse_graph(graph)
assert expected_err in str(cm.value)


def test_parse_graph_simple():
Expand Down

0 comments on commit 7cd0bce

Please sign in to comment.