Преглед изворни кода

[frontend] Update the handle after execute with has_result_set from subsequent check status calls

This takes care of an issue where editor v2 UI won't fetch results if the execute call has the attribute has_result_set initially set to false.
Johan Åhlén пре 4 година
родитељ
комит
67fd25e3dd

+ 1 - 0
desktop/core/src/desktop/js/apps/editor/execution/api.ts

@@ -143,6 +143,7 @@ export interface ExecuteStatusApiResponse {
   result?: ResultApiResponse; // For streaming
   status: string;
   message?: string;
+  has_result_set?: boolean;
 }
 
 export interface ExecuteApiOptions {

+ 53 - 0
desktop/core/src/desktop/js/apps/editor/execution/sqlExecutable.test.ts

@@ -144,6 +144,59 @@ describe('sqlExecutable.js', () => {
     expect(subject.status).toEqual(ExecutionStatus.available);
   });
 
+  it('should update the handle on check status after execute', async () => {
+    const subject = createSubject(SELECT_STATEMENT, undefined, 'impala');
+
+    let statusResolve: (data: unknown) => void;
+    const statusPromise = new CancellablePromise<unknown>(resolve => {
+      statusResolve = resolve;
+    });
+
+    jest.spyOn(ApiUtils, 'post').mockImplementation((url: string): CancellablePromise<unknown> => {
+      if (url.indexOf('/create_session') !== -1) {
+        return CancellablePromise.resolve({ session: { type: 'foo' } });
+      }
+      if (url.indexOf('/execute') !== -1) {
+        expect(url).toContain('/execute/impala');
+        return CancellablePromise.resolve({
+          handle: {
+            has_result_set: false
+          },
+          history_id: 1,
+          history_uuid: 'some-history_uuid',
+          history_parent_uuid: 'some_history_parent_uuid'
+        });
+      }
+      if (url.indexOf('/check_status') !== -1) {
+        statusResolve({
+          query_status: { status: ExecutionStatus.available, has_result_set: true }
+        });
+        return statusPromise;
+      }
+      if (url.indexOf('/get_logs') !== -1) {
+        return CancellablePromise.resolve({ status: 0, logs: '' });
+      }
+
+      if (url.indexOf('/fetch_result_data')) {
+        return CancellablePromise.resolve({});
+      }
+      fail('fail for URL: ' + url);
+      throw new Error('Did not find URL: ' + url);
+    });
+
+    expect(subject.status).toEqual(ExecutionStatus.ready);
+
+    await subject.execute();
+
+    expect(subject.status).not.toEqual(ExecutionStatus.available);
+    expect(subject.handle?.has_result_set).toBeFalsy();
+
+    await statusPromise;
+
+    expect(subject.status).toEqual(ExecutionStatus.available);
+    expect(subject.handle?.has_result_set).toBeTruthy();
+  });
+
   // xit('should set the correct status after failed execute', done => {
   //   const subject = createSubject('SELECT * FROM customers');
   //

+ 7 - 5
desktop/core/src/desktop/js/apps/editor/execution/sqlExecutable.ts

@@ -288,12 +288,10 @@ export default class SqlExecutable {
 
       if (this.handle && this.handle.has_result_set && this.handle.sync) {
         this.result = new ExecutionResult(this);
-        if (this.handle.sync) {
-          if (this.handle.result) {
-            this.result.handleResultResponse(this.handle.result);
-          }
-          this.result.fetchRows();
+        if (this.handle.result) {
+          this.result.handleResultResponse(this.handle.result);
         }
+        this.result.fetchRows();
       }
 
       if (this.executor.isSqlAnalyzerEnabled && this.history) {
@@ -327,6 +325,10 @@ export default class SqlExecutable {
 
     const queryStatus = await checkExecutionStatus({ executable: this });
 
+    if (this.handle && typeof queryStatus.has_result_set !== 'undefined') {
+      this.handle.has_result_set = queryStatus.has_result_set;
+    }
+
     switch (queryStatus.status) {
       case ExecutionStatus.success:
         this.executeEnded = Date.now();

+ 48 - 44
desktop/core/src/desktop/js/catalog/api.ts

@@ -338,52 +338,54 @@ const whenAvailable = (options: {
   snippetJson: string;
   silenceErrors?: boolean;
 }) =>
-  new CancellablePromise<{ status?: string }>(async (resolve, reject, onCancel) => {
-    let promiseToCancel: Cancellable | undefined;
-    let cancelled = false;
-    onCancel(() => {
-      cancelled = true;
-      if (promiseToCancel) {
-        promiseToCancel.cancel();
-      }
-    });
-
-    const checkStatusPromise = post<{ query_status?: { status?: string } }>(
-      CHECK_STATUS_URL,
-      {
-        notebook: options.notebookJson,
-        snippet: options.snippetJson,
-        cluster: (options.entry.compute && JSON.stringify(options.entry.compute)) || '""'
-      },
-      { silenceErrors: options.silenceErrors }
-    );
-    try {
-      promiseToCancel = checkStatusPromise;
-      const response = await checkStatusPromise;
-
-      if (response && response.query_status && response.query_status.status) {
-        const status = response.query_status.status;
-        if (status === 'available') {
-          resolve(response.query_status);
-        } else if (status === 'running' || status === 'starting' || status === 'waiting') {
-          await sleep(500);
-          try {
-            if (!cancelled) {
-              const whenPromise = whenAvailable(options);
-              promiseToCancel = whenPromise;
-              resolve(await whenPromise);
-              return;
-            }
-          } catch (err) {}
+  new CancellablePromise<{ status?: string; has_result_set?: boolean }>(
+    async (resolve, reject, onCancel) => {
+      let promiseToCancel: Cancellable | undefined;
+      let cancelled = false;
+      onCancel(() => {
+        cancelled = true;
+        if (promiseToCancel) {
+          promiseToCancel.cancel();
         }
-        reject(response.query_status);
-      } else {
-        reject('Cancelled');
+      });
+
+      const checkStatusPromise = post<{ query_status?: { status?: string } }>(
+        CHECK_STATUS_URL,
+        {
+          notebook: options.notebookJson,
+          snippet: options.snippetJson,
+          cluster: (options.entry.compute && JSON.stringify(options.entry.compute)) || '""'
+        },
+        { silenceErrors: options.silenceErrors }
+      );
+      try {
+        promiseToCancel = checkStatusPromise;
+        const response = await checkStatusPromise;
+
+        if (response && response.query_status && response.query_status.status) {
+          const status = response.query_status.status;
+          if (status === 'available') {
+            resolve(response.query_status);
+          } else if (status === 'running' || status === 'starting' || status === 'waiting') {
+            await sleep(500);
+            try {
+              if (!cancelled) {
+                const whenPromise = whenAvailable(options);
+                promiseToCancel = whenPromise;
+                resolve(await whenPromise);
+                return;
+              }
+            } catch (err) {}
+          }
+          reject(response.query_status);
+        } else {
+          reject('Cancelled');
+        }
+      } catch (err) {
+        reject(err);
       }
-    } catch (err) {
-      reject(err);
     }
-  });
+  );
 
 export const fetchSample = ({
   entry,
@@ -481,7 +483,9 @@ export const fetchSample = ({
           reject();
           return;
         }
-
+        if (queryResult.result?.handle && typeof resultStatus.has_result_set !== 'undefined') {
+          queryResult.result.handle.has_result_set = resultStatus.has_result_set;
+        }
         snippetJson = JSON.stringify(queryResult);
         // eslint-disable-next-line @typescript-eslint/ban-ts-comment
         // @ts-ignore