mengw15 commented on code in PR #7154:
URL: https://github.com/apache/texera/pull/7154#discussion_r3695465345
##########
common/workflow-operator/src/main/scala/org/apache/texera/amber/operator/loop/LoopOpDesc.scala:
##########
@@ -94,7 +94,14 @@ abstract class LoopOpDesc extends LogicalOp {
operatorName,
operatorDescription,
OperatorGroupConstants.CONTROL_GROUP,
- inputPorts = List(InputPort()),
+ // A loop operator takes exactly one link on its input port. Every reader
+ // on a materialized input port replays that port's states independently,
+ // so a second link would deliver the loop state twice per iteration; a
+ // Loop Start additionally needs a single reader for the scheduler to
+ // resolve its bookkeeping URIs from. Declaring it here is what makes the
+ // GUI refuse to draw the second link, instead of the plan being rejected
+ // only at StartWorkflow (discussion #6966).
+ inputPorts = List(InputPort(disallowMultiLinks = true)),
Review Comment:
Checked the backstop this leans on: the `require` in
`WorkflowExecutionManager` sits under `filter(_.isLoopStart)`, so it only
covers Loop Start. A Loop End with two inbound links has no runtime guard at
all today — it would silently double-consume rather than fail at
`StartWorkflow`. So the Loop End half of this is the part closing a gap with no
safety net, and for programmatically built plans (which is what the
discussion's author does) that case stays unguarded once the GUI is bypassed. A
matching `require` for Loop End might be worth a follow-up. Not blocking.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
