#7568 fix SparkUI connection status widget in multiple notebooks

twosigma · Jun 26, 2018 · e3f86d5 · e3f86d5
1 parent e35dd66
commit e3f86d5
Show file tree

Hide file tree

Showing 2 changed files with 36 additions and 26 deletions.
diff --git a/js/notebook/src/SparkUI.ts b/js/notebook/src/SparkUI.ts
@@ -42,11 +42,11 @@ export class SparkUIView extends widgets.VBoxView {
   sparkStats: Widget;
   connectionStatusElement: HTMLElement;
 
+  private api: BeakerXApi;
   private sparkAppId: string;
   private sparkUiWebUrl: string;
   private sparkMasterUrl: string;
   private apiCallIntervalId: Timer;
-  private toolbarStatusContainer: HTMLElement|null;
   private connectionLabelActive: HTMLElement;
   private connectionLabelMemory: HTMLElement;
   private connectionLabelDead: HTMLElement;
@@ -62,6 +62,7 @@ export class SparkUIView extends widgets.VBoxView {
     this.openExecutors = this.openExecutors.bind(this);
     this.updateChildren = this.updateChildren.bind(this);
     this.toggleExecutorConfigInputs = this.toggleExecutorConfigInputs.bind(this);
+    this.getMetrict = this.getMetrict.bind(this);
 
     this.toolbarSparkConnectionStatus = new ToolbarSparkConnectionStatus(this);
   }
@@ -77,7 +78,6 @@ export class SparkUIView extends widgets.VBoxView {
   public update(): void {
     super.update();
 
-    this.connectToApi();
     this.addSparkMetricsWidget();
     this.handleLocalMasterUrl();
     this.updateChildren();
@@ -214,6 +214,7 @@ export class SparkUIView extends widgets.VBoxView {
                     this.handleLocalMasterUrl();
                     this.toolbarSparkConnectionStatus.append();
                     this.addSparkUrls();
+                    this.connectToApi();
                     this.handleFormState();
                     this.toggleExecutorConfigInputs();
                     this.setupTooltips();
@@ -267,13 +268,10 @@ export class SparkUIView extends widgets.VBoxView {
     this.sparkStats.node.style.marginRight = `${294 - (this.sparkStats.node.offsetWidth + this.connectionStatusElement.offsetWidth)}px`;
   }
 
-  private connectToApi() {
+  private setApi() {
     let baseUrl;
-    let api;
-
-    this.sparkAppId = this.model.get('sparkAppId');
 
-    if (!this.sparkAppId) {
+    if (this.api) {
       return;
     }
 
@@ -285,31 +283,41 @@ export class SparkUIView extends widgets.VBoxView {
       baseUrl = `${window.location.origin}/`;
     }
 
-    api = new BeakerXApi(baseUrl);
-    this.setApiCallInterval(api);
+    this.api = new BeakerXApi(baseUrl);
   }
 
-  private setApiCallInterval(api: BeakerXApi): void {
-    const getMetrict = async () => {
-      try {
-        let sparkUrl = `${api.getApiUrl('sparkmetrics/executors')}/${this.sparkUiWebUrl}/${this.sparkAppId}`;
-        const response = await fetch(sparkUrl, { method: 'GET', credentials: 'include' });
+  private connectToApi() {
+    this.setApi();
+    this.setApiCallInterval();
+  }
 
-        if (!response.ok) {
-          this.toolbarSparkConnectionStatus.destroy();
-          return this.clearApiCallInterval();
-        }
+  private setApiCallInterval(): void {
+    this.clearApiCallInterval();
+    this.sparkAppId = this.model.get('sparkAppId');
 
-        const data = await response.json();
-        this.updateMetrics(data);
-      } catch(error) {
+    if (!this.sparkUiWebUrl || !this.sparkAppId) {
+      return;
+    }
+
+    this.apiCallIntervalId = setInterval(this.getMetrict, 1000);
+  }
+
+  private async getMetrict() {
+    try {
+      let sparkUrl = `${this.api.getApiUrl('sparkmetrics/executors')}?sparkAppId=${this.sparkAppId}&sparkUiWebUrl=${this.sparkUiWebUrl}`;
+      const response = await fetch(sparkUrl, { method: 'GET', credentials: 'include' });
+
+      if (!response.ok) {
         this.toolbarSparkConnectionStatus.destroy();
-        this.clearApiCallInterval();
+        return this.clearApiCallInterval();
       }
-    };
 
-    this.clearApiCallInterval();
-    this.apiCallIntervalId = setInterval(getMetrict, 1000);
+      const data = await response.json();
+      this.updateMetrics(data);
+    } catch(error) {
+      this.toolbarSparkConnectionStatus.destroy();
+      this.clearApiCallInterval();
+    }
   }
 
   private clearApiCallInterval() {

diff --git a/js/notebook/src/sparkUI/toolbarSparkConnectionStatus.ts b/js/notebook/src/sparkUI/toolbarSparkConnectionStatus.ts
@@ -42,7 +42,9 @@ export class ToolbarSparkConnectionStatus {
   }
 
   clear() {
-    this.toolbarSparkStats.node.innerHTML = '';
+    if (this.toolbarSparkStats) {
+      this.toolbarSparkStats.node.innerHTML = '';
+    }
   }
 
   propagateToolbarWidget() {