yangzhang75 commented on code in PR #8440:
URL: https://github.com/apache/texera/pull/8440#discussion_r3948353181
##########
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());
Review Comment:
Fixed. The form now keeps the selected unit and gates Run on write access to
it: runButtonState returns a disabled "No access" when the chosen unit's
accessPrivilege is not WRITE, matching the canvas (menu.component.html:430). A
READ/NONE-shared unit now reaches a disabled button instead of sending an
execution request, so the save() comment about execution being gated on
computing-unit access is accurate. Added tests (No access for a READ unit, plus
the wiring that stores the selected unit).
##########
frontend/src/app/workspace/component/workflow-form/workflow-form.component.ts:
##########
@@ -511,6 +616,154 @@ export class WorkflowFormComponent implements OnInit,
OnDestroy {
return rendered.resolved.binding.id;
}
+ //
---------------------------------------------------------------------------
+ // Instruction: the author's one piece of guidance, shown as rendered
markdown
+ //
---------------------------------------------------------------------------
+
+ public get hasInstruction(): boolean {
+ return this.instructionBody.trim().length > 0;
+ }
+
+ private async renderInstruction(): Promise<void> {
+ // Capture the body this render is for: parsing can resolve on a later
microtask, and a fresh
+ // readConfig() may start another render meanwhile. If the configured body
changed while we were
+ // parsing, this result is stale -- drop it so the newer render's output
stands.
+ const body = this.instructionBody;
+ const html = body.trim() ? await
Promise.resolve(this.markdownService.parse(body)) : "";
+ if (body !== this.instructionBody) {
+ return;
+ }
+ this.instructionPreviewHtml = html;
+ this.cdr.detectChanges();
+ }
+
+ public toggleInstruction(): void {
+ this.instructionOpen = !this.instructionOpen;
+ }
+
+ //
---------------------------------------------------------------------------
+ // Running the same workflow the canvas runs, through the same execute/kill
service. The canvas
+ // wraps its run with completion-email options
(executeWorkflowWithEmailNotification); this page
+ // runs plainly (executeWorkflow), so a form-started run does not send that
email.
+ //
---------------------------------------------------------------------------
+
+ public get isRunning(): boolean {
+ return (
+ this.executionState !== ExecutionState.Uninitialized &&
+ this.executionState !== ExecutionState.Completed &&
+ this.executionState !== ExecutionState.Failed &&
+ this.executionState !== ExecutionState.Killed &&
+ this.executionState !== ExecutionState.Terminated
+ );
+ }
+
+ /**
+ * A unit is picked but its socket is still coming up -- the same window the
operator canvas shows
+ * "Connecting" and disables its run button. Read from the exact condition
the canvas uses
+ * (menu.component's getRunButtonBehavior), so the two stay in step.
+ */
+ public get isConnecting(): boolean {
+ return (
+ this.computingUnitStatus !== ComputingUnitState.NoComputingUnit &&
!this.workflowWebsocketService.isConnected
+ );
+ }
+
+ /** No unit chosen yet: the button shows a disabled "Connect" hint and the
unit is picked in the
+ * embedded selector -- unlike the canvas, where the Connect button is
itself the click target. */
+ public get hasNoComputingUnit(): boolean {
+ return this.computingUnitStatus === ComputingUnitState.NoComputingUnit;
+ }
+
+ /**
+ * The Run button's label, icon and disabled state. It shares the operator
canvas's disable
+ * conditions -- an invalid or empty workflow, a unit still connecting, or
no unit chosen each
+ * disable it and say why -- but deliberately simplifies the execution
states a reader needs down
+ * to Run and Stop, with no pause/resume: while a run is in flight the
button stops (kills) it,
+ * otherwise it runs. (The canvas offers Pause/Resume/Submitting and a
clickable Connect; a form
+ * reader does not, and picks the unit in the embedded selector instead.)
+ */
+ public get runButtonState(): { label: string; icon: string; disabled:
boolean } {
+ if (this.isRunning) {
+ return { label: "Stop", icon: "stop", disabled: false };
+ }
+ if (!this.isWorkflowValid) {
+ return { label: "Invalid", icon: "warning", disabled: true };
+ }
+ if (this.isWorkflowEmpty) {
+ return { label: "Empty", icon: "info-circle", disabled: true };
+ }
+ if (this.isConnecting) {
+ return { label: "Connecting", icon: "loading", disabled: true };
+ }
+ if (this.hasNoComputingUnit) {
+ return { label: "Connect", icon: "plus-circle", disabled: true };
+ }
+ return { label: "Run", icon: "caret-right", disabled: false };
+ }
Review Comment:
Fixed. runButtonState now checks the connecting/disconnected state BEFORE
isRunning, so if the socket drops during a run the button stays a disabled
"Connecting" rather than an enabled Stop that would send killWorkflow() through
a dead socket (matching the canvas, which disables its controls in this state).
Added a test for a mid-run socket drop.
##########
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:
Agreed, and fixed: the execution-state handler now uses markForCheck()
instead of detectChanges(). As you say, this is the one subscription the page
cannot afford to lose -- a synchronous pass thrown out of by an unrelated
NG0100 would have killed it and frozen the Run button on a stale state. Thanks
for catching it.
--
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]
