Xiao-zhen-Liu commented on code in PR #3999:
URL: https://github.com/apache/texera/pull/3999#discussion_r2467827135


##########
frontend/src/app/workspace/component/workflow-editor/workflow-editor.component.ts:
##########
@@ -376,6 +376,18 @@ export class WorkflowEditorComponent implements OnInit, 
AfterViewInit, OnDestroy
         .filter(region => region.operators.includes(operator))
         .forEach(region => this.updateRegionElement(region.regionElement, 
region.operators));
     });
+
+    this.executeWorkflowService
+      .getRegionStateStream()
+      .pipe(untilDestroyed(this))
+      .subscribe(region => {
+        const colorMap: Record<string, string> = {
+          ExecutingDependeePortsPhase: "rgba(244,67,54,0.2)",

Review Comment:
   You mentioned "no regions will appear in the red state yet", but that is not 
true. Some regions will actually go through an `ExecutingDependeePortsPhase`, 
e.g., a region containing a `Hash Join`'s Probe. You can try it on such a 
workflow. 
   
   Also the red color might not be appropriate as it usually indicates error.



##########
frontend/src/app/workspace/component/workflow-editor/workflow-editor.component.ts:
##########
@@ -376,6 +376,18 @@ export class WorkflowEditorComponent implements OnInit, 
AfterViewInit, OnDestroy
         .filter(region => region.operators.includes(operator))
         .forEach(region => this.updateRegionElement(region.regionElement, 
region.operators));
     });
+
+    this.executeWorkflowService

Review Comment:
   Add some comment about what this code block does?
   Also this method is `handleRegionUpdate`, which is supposed to only handle 
the `RegionUpdate` event. Here you are handling the `RegionState` event. To 
make things clearer, I suggest you either change the name of this method, or 
move this new logic into its own method.



-- 
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]

Reply via email to