aglinxinyuan commented on code in PR #4206:
URL: https://github.com/apache/texera/pull/4206#discussion_r3291073867
##########
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:
Reshaped in 2150e648a3 — backend keeps the coercion, frontend mirrors what
the backend acknowledges:
* New event `WorkflowSettingsUpdateEvent(executionMode,
dataTransferBatchSize)` registered in `TexeraWebSocketEvent`.
* `WorkflowExecutionService` still rewrites `executionMode` to
`MATERIALIZED` when the workflow contains a Loop Start, but now also pushes the
(possibly-coerced) effective settings back to the frontend via this event. A
new `pushEvent` callback param on the constructor; `WorkflowService` wires it
to the existing one-shot event subject that `connect()` already forwards to the
websocket `onNext`.
* Frontend handler in `ExecuteWorkflowService` calls
`WorkflowActionService.setWorkflowSettings(...)`. `setWorkflowSettings` now
fires a new `workflowSettingsChanged()` Observable, which `SettingsComponent`
subscribes to and uses to re-patch the form. No more loop-detection logic in
the frontend — the radio just shows whatever the backend acknowledged.
Net change: ~155 lines of frontend loop-detection scaffolding removed,
replaced with ~30 lines of websocket plumbing. Frontend tests: 483 passed, plus
one new case asserting the form patches when the server pushes a coerced
settings update.
##########
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:
Reworked in 36c4ff4501 — UI and engine now agree, no error message.
**Frontend** (`settings.component`): on every `workflowChanged` event the
panel walks the operators; if any is `LoopStart` / `LoopEnd` it forces
`executionMode` to `MATERIALIZED`, disables the radio group, and shows a
one-shot info toast the first time it flips the selection (subsequent passes
don't re-notify). When the last loop operator is removed, the radio group is
re-enabled. New tests cover all six branches (LoopStart coerces, LoopEnd
coerces, removal re-enables, single toast, no toast when already-MATERIALIZED,
no-op without a loop).
**Backend** (`WorkflowExecutionService`): restored the silent `MATERIALIZED`
coercion as a safety net for non-frontend callers (direct API submissions,
older clients). With the frontend mirroring the rule, the backend coercion is
normally a no-op; if it does fire, both sides land on MATERIALIZED rather than
diverging.
--
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]
