Skip to content
Open
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
Original file line number Diff line number Diff line change
Expand Up @@ -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)),

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

// Loop End reuses its output storage across region re-executions (it
// accumulates across the iterations of its own loop); the flag is
// declared on the output port and the region scheduler reads it there.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,14 @@ class LoopEndOpDescSpec extends AnyFlatSpec with LoopOpDescSpecMixin {
info.outputPorts should have length 1
}

it should "disallow more than one link into its input port" in {
// Each reader on an input port replays the loop state independently, so a
// second link would make this Loop End consume the same iteration twice
// (double `update`, double back-edge). Declaring it on the port stops the
// GUI from drawing the second link (discussion #6966).
desc().operatorInfo.inputPorts.head.disallowMultiLinks shouldBe true
}

"LoopEndOpDesc.generatePythonCode" should "wrap user inputs in the base64 decode template" in {
// Distinct sentinels so we know the codegen wires the right user
// field into the right `decode_python_template` site. If `condition`
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,15 @@ class LoopStartOpDescSpec extends AnyFlatSpec with LoopOpDescSpecMixin {
info.outputPorts should have length 1
}

it should "disallow more than one link into its input port" in {
// The runtime resolves the loop's bookkeeping URIs from this port's single
// reader (WorkflowExecutionManager requires exactly one storage pair), and
// every extra reader would replay the loop state again. Declaring it on
// the port is what stops the GUI from drawing a second link at all, rather
// than failing at StartWorkflow (discussion #6966).
desc().operatorInfo.inputPorts.head.disallowMultiLinks shouldBe true
}

"LoopStartOpDesc.generatePythonCode" should "wrap user inputs in the base64 decode template" in {
// Distinct sentinels prove the codegen routes the right user field
// through the encode pipeline (not accidentally swapped) and that
Expand Down
Loading