yangjunhan commented on code in PR #21447:
URL: https://github.com/apache/flink/pull/21447#discussion_r1059229187
##########
flink-runtime-web/web-dashboard/src/app/pages/job/overview/flamegraph/job-overview-drawer-flamegraph.component.ts:
##########
@@ -100,9 +118,38 @@ export class JobOverviewDrawerFlameGraphComponent
implements OnInit, OnDestroy {
);
}
+ private requestRunningSubtasks(): void {
+ this.jobLocalService
+ .jobWithVertexChanges()
+ .pipe(
+ tap(data => (this.selectedVertex = data.vertex)),
+ mergeMap(data => {
+ return this.jobService.loadSubTasks(data.job.jid, data.vertex!.id);
+ }),
+ takeUntil(this.destroy$)
+ )
+ .subscribe(
+ data => {
+ const runningSubtasks = data?.subtasks
Review Comment:
Can `data` be null or undefined in this case? If not, you can skip the
optional chaining (`?.`).
`runningSubtasks` can be undefined due to the use of optional chaining. This
causes the spread operator on line 136 unsafe. Instead, you should wrap it with
a IF statement checking whether `runningSubtasks` is not undefined.
##########
flink-runtime-web/web-dashboard/src/app/pages/job/overview/flamegraph/job-overview-drawer-flamegraph.component.ts:
##########
@@ -100,9 +118,38 @@ export class JobOverviewDrawerFlameGraphComponent
implements OnInit, OnDestroy {
);
}
+ private requestRunningSubtasks(): void {
+ this.jobLocalService
+ .jobWithVertexChanges()
+ .pipe(
+ tap(data => (this.selectedVertex = data.vertex)),
+ mergeMap(data => {
+ return this.jobService.loadSubTasks(data.job.jid, data.vertex!.id);
+ }),
+ takeUntil(this.destroy$)
+ )
+ .subscribe(
+ data => {
+ const runningSubtasks = data?.subtasks
+ .filter(subtaskInfo => subtaskInfo.status === 'RUNNING')
+ .map(subtaskInfo => subtaskInfo.subtask.toString());
+ this.listOfRunningSubtasks = [this.allSubtasks, ...runningSubtasks];
+ this.cdr.markForCheck();
+ },
+ () => {
+ this.cdr.markForCheck();
+ }
+ );
+ }
+
+ public selectSubtask(subtaskIndex: string): void {
+ this.destroy$.next();
+ this.subtaskIndex = subtaskIndex;
Review Comment:
Miss a markForCheck here
##########
flink-runtime-web/web-dashboard/src/app/pages/job/overview/flamegraph/job-overview-drawer-flamegraph.component.ts:
##########
@@ -100,9 +118,38 @@ export class JobOverviewDrawerFlameGraphComponent
implements OnInit, OnDestroy {
);
}
+ private requestRunningSubtasks(): void {
+ this.jobLocalService
+ .jobWithVertexChanges()
+ .pipe(
+ tap(data => (this.selectedVertex = data.vertex)),
+ mergeMap(data => {
+ return this.jobService.loadSubTasks(data.job.jid, data.vertex!.id);
+ }),
+ takeUntil(this.destroy$)
+ )
+ .subscribe(
+ data => {
+ const runningSubtasks = data?.subtasks
+ .filter(subtaskInfo => subtaskInfo.status === 'RUNNING')
+ .map(subtaskInfo => subtaskInfo.subtask.toString());
+ this.listOfRunningSubtasks = [this.allSubtasks, ...runningSubtasks];
+ this.cdr.markForCheck();
+ },
+ () => {
+ this.cdr.markForCheck();
Review Comment:
This line alone is useless if no view data is changed. Do something in the
error callback if you want such as `this.listOfRunningSubtasks = [];`
--
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: issues-unsubscr...@flink.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
