aglinxinyuan commented on code in PR #4206:
URL: https://github.com/apache/texera/pull/4206#discussion_r3309041098
##########
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]))
{
+ request.workflowSettings.copy(executionMode = ExecutionMode.MATERIALIZED)
+ } else {
+ request.workflowSettings
Review Comment:
Fixed in 30bf1cd136 by failing loud instead of silently coercing.
`WorkflowExecutionService.validateExecutionMode` now throws an
`IllegalArgumentException` when the plan contains an operator that requires
materialized execution but the requested mode isn't MATERIALIZED:
> This workflow contains operators that require MATERIALIZED execution mode
(e.g. Loop Start / Loop End). Please set the execution mode to Materialized in
the workflow settings and run again.
`WorkflowService` already wraps the constructor in `try { ... } catch { case
e => errorHandler(e) }`, so this surfaces to the UI as a fatal workflow error —
the user is told to switch the mode and re-run, rather than the UI and engine
silently disagreeing. The requirement is still keyed off the generic
`LogicalOp.requiresMaterializedExecution` flag (no operator-class dependency),
and the spec covers it: loop + non-MATERIALIZED throws (including a
LoopEnd-only plan), while a loop already set to MATERIALIZED, a non-loop op,
and an empty plan all pass.
--
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]
