aglinxinyuan commented on code in PR #4206:
URL: https://github.com/apache/texera/pull/4206#discussion_r3308899817
##########
amber/src/main/scala/org/apache/texera/web/service/WorkflowExecutionService.scala:
##########
@@ -66,7 +67,12 @@ class WorkflowExecutionService(
) extends SubscriptionManager
with LazyLogging {
- workflowContext.workflowSettings = request.workflowSettings
+ workflowContext.workflowSettings =
+ if (request.logicalPlan.operators.exists(_.isInstanceOf[LoopStartOpDesc]))
{
Review Comment:
Both addressed in 1848ce00fb.
**1. Generic service no longer depends on a specific operator class.** Added
`LogicalOp.requiresMaterializedExecution` (default `false`); `LoopStartOpDesc`
and `LoopEndOpDesc` override it to `true`. `WorkflowExecutionService` now
checks that flag instead of `isInstanceOf[LoopStartOpDesc]` — the
`LoopStartOpDesc` import is gone, and any future operator that needs
materialized edges just sets the flag.
**2. The rule is now tested.** Extracted it into a pure, side-effect-free
`WorkflowExecutionService.resolveWorkflowSettings(operators, requested)` (the
constructor body just delegates), and added `WorkflowExecutionServiceSpec`:
* `requiresMaterializedExecution` is true for `LoopStart`/`LoopEnd`, false
for a non-loop op (`SleepOpDesc`);
* loop present (incl. a plan mixing loop + non-loop ops) → coerced to
MATERIALIZED;
* `LoopEnd` alone also coerces;
* non-loop op and empty plan pass through unchanged;
* idempotent when the user already chose MATERIALIZED;
* only the `executionMode` field changes (other settings preserved).
7 tests, all passing.
--
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]
