mengw15 commented on code in PR #8440:
URL: https://github.com/apache/texera/pull/8440#discussion_r3947718467
##########
frontend/src/app/workspace/component/workflow-form/workflow-form.component.ts:
##########
@@ -164,6 +200,70 @@ export class WorkflowFormComponent implements OnInit,
OnDestroy {
this.wid = wid;
this.load(wid);
+ // The run clock, reusing the operator canvas's source outright rather
than timing anything
+ // here: the engine is the only thing that knows when the run really
began, so a stopwatch
+ // started at the click would drift and would be wrong after a reload.
+ this.workflowWebsocketService
+ .subscribeToEvent("ExecutionDurationUpdateEvent")
+ .pipe(
+ tap(event => (this.executionDuration = event.duration)),
+ switchMap(event => (event.isRunning ? timer(1000, 1000) : EMPTY)),
+ untilDestroyed(this)
+ )
+ .subscribe(() => {
+ this.executionDuration += 1000;
+ this.cdr.markForCheck();
+ });
+
+ // The run button's state is read from getters, so a change in
unit/connection/validity has to
+ // repaint the view. markForCheck, not detectChanges: a synchronous pass
can be thrown out of by
+ // an unrelated component's NG0100, killing the subscription.
+ this.computingUnitStatusService
+ .getSelectedComputingUnit()
+ .pipe(untilDestroyed(this))
+ .subscribe(() => this.cdr.markForCheck());
+ this.computingUnitStatusService
+ .getStatus()
+ .pipe(untilDestroyed(this))
+ .subscribe(status => {
+ this.computingUnitStatus = status;
+ this.cdr.markForCheck();
+ });
+ this.workflowWebsocketService
+ .getConnectionStatusStream()
+ .pipe(untilDestroyed(this))
+ .subscribe(() => this.cdr.markForCheck());
+ // Validity from the canvas's own stream, so a broken graph disables Run
("Invalid") here
+ // exactly as it does there.
+ this.validationWorkflowService
+ .getWorkflowValidationErrorStream()
+ .pipe(untilDestroyed(this))
+ .subscribe(value => {
+ this.isWorkflowEmpty = value.workflowEmpty;
+ this.isWorkflowValid = Object.keys(value.errors).length === 0;
+ this.cdr.markForCheck();
+ });
+
+ this.executeWorkflowService
+ .getExecutionStateStream()
+ .pipe(untilDestroyed(this))
+ .subscribe(({ current }) => {
+ this.executionState = current.state;
+ // Surface a failed run. Without this the spinner just stops and the
form gives zero
+ // feedback -- the opposite of what a reader needs. onRun() clears
runError before the next
+ // run, so a stale error never lingers.
+ if (current.state === ExecutionState.Failed) {
+ // A required input left empty is by far the commonest reason a run
fails here, and the
+ // engine reports it as an opaque "... is not contained in the
schema". Answer with the
+ // same word the field itself already shows ("required"), so the two
messages are
+ // consistent -- and it covers every operator, not just this one.
+ this.runError = this.hasEmptyRequiredInputs()
+ ? "Run failed: please fill in the required fields."
+ :
this.friendlyRunError(current.errorMessages?.[0]?.message?.trim() ?? "");
+ }
+ this.cdr.detectChanges();
Review Comment:
The rationale above (markForCheck, "a synchronous pass can be thrown out of
by an unrelated component's NG0100, killing the subscription") applies most of
all to this subscription — if this `detectChanges()` throws, the
execution-state stream dies and the Run button is stuck on a stale state with
no error surfaced, which is the one subscription this page cannot afford to
lose. Worth using `markForCheck()` here too, unless there is a reason this one
needs the synchronous 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]
