Utils to convert metadata api from callback paradigm to promise parad…

…igm (#2153)
kubeflow · Sep 18, 2019 · 4d9a4e8 · 4d9a4e8
1 parent 2572013
commit 4d9a4e8
Show file tree

Hide file tree

Showing 3 changed files with 80 additions and 53 deletions.
diff --git a/frontend/src/lib/Apis.ts b/frontend/src/lib/Apis.ts
@@ -19,7 +19,7 @@ import { ApiPipeline, PipelineServiceApi } from '../apis/pipeline';
 import { RunServiceApi } from '../apis/run';
 import { ApiVisualization, VisualizationServiceApi } from '../apis/visualization';
 import { PlotType } from '../components/viewers/Viewer';
-import { MetadataStoreServiceClient } from '../generated/src/apis/metadata/metadata_store_service_pb_service';
+import { MetadataStoreServiceClient, ServiceError, UnaryResponse } from '../generated/src/apis/metadata/metadata_store_service_pb_service';
 import * as Utils from './Utils';
 import { StoragePath } from './WorkflowParser';
 
@@ -70,7 +70,30 @@ export interface BuildInfo {
 }
 
 let customVisualizationsAllowed: boolean;
+
+type Callback<R> = (err: ServiceError | null, res: R | null) => void;
+type MetadataApiMethod<T, R> = (request: T, callback: Callback<R>) => UnaryResponse;
+type PromiseBasedMetadataApiMethod<T, R> = (request: T) => Promise<{ response: R | null, error: ServiceError | null }>;
+
+/**
+ * Converts a callback based api method to promise based api method.
+ */
+function makePromiseApi<T, R>(apiMethod: MetadataApiMethod<T, R>): PromiseBasedMetadataApiMethod<T, R> {
+  return (request: T) => new Promise((resolve, reject) => {
+    const handler = (error: ServiceError | null, response: R | null) => {
+      // resolve both response and error to keep type information
+      resolve({ response, error });
+    };
+    apiMethod(request, handler);
+  });
+}
 const metadataServiceClient = new MetadataStoreServiceClient('');
+// TODO: add all other api methods we need here.
+const metadataServicePromiseClient = {
+  getEventsByArtifactIDs: makePromiseApi(metadataServiceClient.getEventsByArtifactIDs.bind(metadataServiceClient)),
+  getEventsByExecutionIDs: makePromiseApi(metadataServiceClient.getEventsByExecutionIDs.bind(metadataServiceClient)),
+  getExecutionsByID: makePromiseApi(metadataServiceClient.getExecutionsByID.bind(metadataServiceClient)),
+};
 
 export class Apis {
 
@@ -233,6 +256,12 @@ export class Apis {
     return metadataServiceClient;
   }
 
+  // It will be a lot of boilerplate to type the following method, omit it here.
+  // tslint:disable-next-line:typedef
+  public static getMetadataServicePromiseClient() {
+    return metadataServicePromiseClient;
+  }
+
   private static _experimentServiceApi?: ExperimentServiceApi;
   private static _jobServiceApi?: JobServiceApi;
   private static _pipelineServiceApi?: PipelineServiceApi;

diff --git a/frontend/src/lib/MetadataUtils.ts b/frontend/src/lib/MetadataUtils.ts
@@ -3,33 +3,29 @@ import { GetEventsByArtifactIDsRequest, GetEventsByArtifactIDsResponse } from '.
 import { Apis } from '../lib/Apis';
 import { formatDateString, serviceErrorToString } from './Utils';
 
-export const getArtifactCreationTime = (artifactId: number): Promise<string> => {
-  return new Promise((resolve, reject) => {
-    const eventsRequest = new GetEventsByArtifactIDsRequest();
-    if (!artifactId) {
-      return reject(new Error('artifactId is empty'));
-    }
+export const getArtifactCreationTime = async (artifactId: number): Promise<string> => {
+  const eventsRequest = new GetEventsByArtifactIDsRequest();
+  if (!artifactId) {
+    throw new Error('artifactId is empty');
+  }
 
-    eventsRequest.setArtifactIdsList([artifactId]);
-    Apis.getMetadataServiceClient().getEventsByArtifactIDs(eventsRequest, (err, res) => {
-      if (err) {
-        return reject(new Error(serviceErrorToString(err)));
-      } else {
-        const data = (res as GetEventsByArtifactIDsResponse).getEventsList().map(event => ({
-          time: event.getMillisecondsSinceEpoch(),
-          type: event.getType() || Event.Type.UNKNOWN,
-        }));
-        // The last output event is the event that produced current artifact.
-        const lastOutputEvent = data.reverse().find(event =>
-          event.type === Event.Type.DECLARED_OUTPUT || event.type === Event.Type.OUTPUT
-        );
-        if (lastOutputEvent && lastOutputEvent.time) {
-          resolve(formatDateString(new Date(lastOutputEvent.time)));
-        } else {
-          // No valid time found, just return empty
-          resolve('');
-        }
-      }
-    });
-  });
+  eventsRequest.setArtifactIdsList([artifactId]);
+  const { error, response } = await Apis.getMetadataServicePromiseClient().getEventsByArtifactIDs(eventsRequest);
+  if (error) {
+    throw new Error(serviceErrorToString(error));
+  }
+  const data = (response as GetEventsByArtifactIDsResponse).getEventsList().map(event => ({
+    time: event.getMillisecondsSinceEpoch(),
+    type: event.getType() || Event.Type.UNKNOWN,
+  }));
+  // The last output event is the event that produced current artifact.
+  const lastOutputEvent = data.reverse().find(event =>
+    event.type === Event.Type.DECLARED_OUTPUT || event.type === Event.Type.OUTPUT
+  );
+  if (lastOutputEvent && lastOutputEvent.time) {
+    return formatDateString(new Date(lastOutputEvent.time));
+  } else {
+    // No valid time found, just return empty
+    return '';
+  }
 };
diff --git a/frontend/src/pages/ExecutionDetails.tsx b/frontend/src/pages/ExecutionDetails.tsx
@@ -86,38 +86,40 @@ export default class ExecutionDetails extends Page<{}, ExecutionDetailsState> {
   }
 
   private async load(): Promise<void> {
-    const getExecutionsRequest = new GetExecutionsByIDRequest();
     const numberId = parseInt(this.id, 10);
     if (isNaN(numberId) || numberId < 0) {
       const error = new Error(`Invalid execution id: ${this.id}`);
       this.showPageError(error.message, error);
       return Promise.reject(error);
     }
 
+    const getExecutionsRequest = new GetExecutionsByIDRequest();
     getExecutionsRequest.setExecutionIdsList([numberId]);
-    Apis.getMetadataServiceClient().getExecutionsByID(getExecutionsRequest, (err, res) => {
-      if (err) {
-        this.showPageError(serviceErrorToString(err));
-        return;
-      }
-
-      if (!res || !res.getExecutionsList().length) {
-        this.showPageError(`No ${this.fullTypeName} identified by id: ${this.id}`);
-        return;
-      }
-
-      if (res.getExecutionsList().length > 1) {
-        this.showPageError(`Found multiple executions with ID: ${this.id}`);
-        return;
-      }
-
-      const execution = res.getExecutionsList()[0];
-
-      const executionName = getResourceProperty(execution, ExecutionProperties.COMPONENT_ID);
-      this.props.updateToolbar({
-        pageTitle: executionName ? executionName.toString() : ''
-      });
-      this.setState({ execution });
+
+    const executionResponse = await Apis.getMetadataServicePromiseClient().getExecutionsByID(getExecutionsRequest);
+
+    if (executionResponse.error) {
+      this.showPageError(serviceErrorToString(executionResponse.error));
+      return;
+    }
+    if (!executionResponse.response || !executionResponse.response.getExecutionsList().length) {
+      this.showPageError(`No ${this.fullTypeName} identified by id: ${this.id}`);
+      return;
+    }
+    if (executionResponse.response.getExecutionsList().length > 1) {
+      this.showPageError(`Found multiple executions with ID: ${this.id}`);
+      return;
+    }
+
+    const execution = executionResponse.response.getExecutionsList()[0];
+
+    const executionName = getResourceProperty(execution, ExecutionProperties.COMPONENT_ID);
+    this.props.updateToolbar({
+      pageTitle: executionName ? executionName.toString() : ''
+    });
+
+    this.setState({
+      execution,
     });
   }
 }