[GitHub] [flink] simplejason commented on a diff in pull request #19712: [FLINK-26207][runtime-web] make web ui's components depend on injected configurable object

[GitBox <gi...@apache.org>]

simplejason commented on code in PR #19712:
URL: https://github.com/apache/flink/pull/19712#discussion_r880371223


##########
flink-runtime-web/web-dashboard/src/app/pages/job-manager/configuration/job-manager-configuration.component.ts:
##########
@@ -26,15 +29,33 @@ import { JobManagerService } from '@flink-runtime-web/services';
   styleUrls: ['./job-manager-configuration.component.less'],
   changeDetection: ChangeDetectionStrategy.OnPush
 })
-export class JobManagerConfigurationComponent implements OnInit {
+export class JobManagerConfigurationComponent implements OnInit, OnDestroy {
   public listOfConfig: Array<{ key: string; value: string }> = [];
+  public loading = true;
+  private destroy$ = new Subject<void>();
+
+  public readonly trackByConfig = (_: number, value: JobManagerConfig): string => {
+    return value.key;
+  };
 
   constructor(private readonly jobManagerService: JobManagerService, private readonly cdr: ChangeDetectorRef) {}
 
   public ngOnInit(): void {
-    this.jobManagerService.loadConfig().subscribe(data => {
-      this.listOfConfig = data.sort((pre, next) => (pre.key > next.key ? 1 : -1));
-      this.cdr.markForCheck();
-    });
+    this.jobManagerService
+      .loadConfig()
+      .pipe(
+        catchError(() => of([] as JobManagerConfig[])),
+        takeUntil(this.destroy$)

Review Comment:
   I think `takeUntil` is not required because Http-based implementation of loadConfig will complete and only excute once.



-- 
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-unsubscribe@flink.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org