You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by mi...@apache.org on 2024/02/13 16:14:31 UTC

(superset) branch 3.1 updated (8831d60bbc -> 4f839ef2d1)

This is an automated email from the ASF dual-hosted git repository.

michaelsmolina pushed a change to branch 3.1
in repository https://gitbox.apache.org/repos/asf/superset.git


 discard 8831d60bbc fix: Drill by with GLOBAL_ASYNC_QUERIES (#27066)
     new 4f839ef2d1 fix: Drill by with GLOBAL_ASYNC_QUERIES (#27066)

This update added new revisions after undoing existing revisions.
That is to say, some revisions that were in the old version of the
branch are not in the new version.  This situation occurs
when a user --force pushes a change and generates a repository
containing something like this:

 * -- * -- B -- O -- O -- O   (8831d60bbc)
            \
             N -- N -- N   refs/heads/3.1 (4f839ef2d1)

You should already have received notification emails for all of the O
revisions, and so the following emails describe only the N revisions
from the common base, B.

Any revisions marked "omit" are not gone; other references still
refer to them.  Any revisions marked "discard" are gone forever.

The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.


Summary of changes:
 superset-frontend/package-lock.json | 44 ++++++++++++++++++++++++++++---------
 1 file changed, 34 insertions(+), 10 deletions(-)


(superset) 01/01: fix: Drill by with GLOBAL_ASYNC_QUERIES (#27066)

Posted by mi...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

michaelsmolina pushed a commit to branch 3.1
in repository https://gitbox.apache.org/repos/asf/superset.git

commit 4f839ef2d182486ea49a5da74fe2f44c1451fa4d
Author: Kamil Gabryjelski <ka...@gmail.com>
AuthorDate: Fri Feb 9 21:49:33 2024 +0100

    fix: Drill by with GLOBAL_ASYNC_QUERIES (#27066)
---
 superset-frontend/package-lock.json                | 44 +++++++++++++-----
 .../src/components/Chart/DrillBy/DrillByModal.tsx  | 11 +++--
 .../src/components/Chart/chartAction.js            | 52 +++++++++++-----------
 .../src/components/Chart/chartActions.test.js      | 43 +++++++++++++++++-
 superset/security/manager.py                       |  6 ++-
 5 files changed, 115 insertions(+), 41 deletions(-)

diff --git a/superset-frontend/package-lock.json b/superset-frontend/package-lock.json
index 6cacdb3a0b..8b371df962 100644
--- a/superset-frontend/package-lock.json
+++ b/superset-frontend/package-lock.json
@@ -5346,6 +5346,11 @@
         "csstype": "^2.5.7"
       }
     },
+    "node_modules/@emotion/serialize/node_modules/csstype": {
+      "version": "2.6.21",
+      "resolved": "https://registry.npmjs.org/csstype/-/csstype-2.6.21.tgz",
+      "integrity": "sha512-Z1PhmomIfypOpoMjRQB70jfvy/wxT50qW08YXO5lMIJkrdq4yOTR+AW7FqutScmB9NkLwxo+jU+kZLbofZZq/w=="
+    },
     "node_modules/@emotion/sheet": {
       "version": "0.9.4",
       "resolved": "https://registry.npmjs.org/@emotion/sheet/-/sheet-0.9.4.tgz",
@@ -19773,6 +19778,11 @@
         "@types/react": "*"
       }
     },
+    "node_modules/@types/react/node_modules/csstype": {
+      "version": "2.6.21",
+      "resolved": "https://registry.npmjs.org/csstype/-/csstype-2.6.21.tgz",
+      "integrity": "sha512-Z1PhmomIfypOpoMjRQB70jfvy/wxT50qW08YXO5lMIJkrdq4yOTR+AW7FqutScmB9NkLwxo+jU+kZLbofZZq/w=="
+    },
     "node_modules/@types/redux-localstorage": {
       "version": "1.0.8",
       "resolved": "https://registry.npmjs.org/@types/redux-localstorage/-/redux-localstorage-1.0.8.tgz",
@@ -28067,11 +28077,6 @@
       "integrity": "sha512-b0tGHbfegbhPJpxpiBPU2sCkigAqtM9O121le6bbOlgyV+NyGyCmVfJ6QW9eRjz8CpNfWEOYBIMIGRYkLwsIYg==",
       "dev": true
     },
-    "node_modules/csstype": {
-      "version": "2.6.9",
-      "resolved": "https://registry.npmjs.org/csstype/-/csstype-2.6.9.tgz",
-      "integrity": "sha512-xz39Sb4+OaTsULgUERcCk+TJj8ylkL4aSVDQiX/ksxbELSqwkgt4d4RD7fovIdgJGSuNYqwZEiVjYY5l0ask+Q=="
-    },
     "node_modules/currencyformatter.js": {
       "version": "1.0.5",
       "resolved": "https://registry.npmjs.org/currencyformatter.js/-/currencyformatter.js-1.0.5.tgz",
@@ -52126,6 +52131,11 @@
         "@emotion/weak-memoize": "0.2.5"
       }
     },
+    "node_modules/react-select/node_modules/csstype": {
+      "version": "2.6.21",
+      "resolved": "https://registry.npmjs.org/csstype/-/csstype-2.6.21.tgz",
+      "integrity": "sha512-Z1PhmomIfypOpoMjRQB70jfvy/wxT50qW08YXO5lMIJkrdq4yOTR+AW7FqutScmB9NkLwxo+jU+kZLbofZZq/w=="
+    },
     "node_modules/react-select/node_modules/dom-helpers": {
       "version": "5.1.4",
       "resolved": "https://registry.npmjs.org/dom-helpers/-/dom-helpers-5.1.4.tgz",
@@ -67639,6 +67649,13 @@
         "@emotion/unitless": "0.7.5",
         "@emotion/utils": "0.11.3",
         "csstype": "^2.5.7"
+      },
+      "dependencies": {
+        "csstype": {
+          "version": "2.6.21",
+          "resolved": "https://registry.npmjs.org/csstype/-/csstype-2.6.21.tgz",
+          "integrity": "sha512-Z1PhmomIfypOpoMjRQB70jfvy/wxT50qW08YXO5lMIJkrdq4yOTR+AW7FqutScmB9NkLwxo+jU+kZLbofZZq/w=="
+        }
       }
     },
     "@emotion/sheet": {
@@ -79607,6 +79624,13 @@
       "requires": {
         "@types/prop-types": "*",
         "csstype": "^2.2.0"
+      },
+      "dependencies": {
+        "csstype": {
+          "version": "2.6.21",
+          "resolved": "https://registry.npmjs.org/csstype/-/csstype-2.6.21.tgz",
+          "integrity": "sha512-Z1PhmomIfypOpoMjRQB70jfvy/wxT50qW08YXO5lMIJkrdq4yOTR+AW7FqutScmB9NkLwxo+jU+kZLbofZZq/w=="
+        }
       }
     },
     "@types/react-dom": {
@@ -86171,11 +86195,6 @@
         }
       }
     },
-    "csstype": {
-      "version": "2.6.9",
-      "resolved": "https://registry.npmjs.org/csstype/-/csstype-2.6.9.tgz",
-      "integrity": "sha512-xz39Sb4+OaTsULgUERcCk+TJj8ylkL4aSVDQiX/ksxbELSqwkgt4d4RD7fovIdgJGSuNYqwZEiVjYY5l0ask+Q=="
-    },
     "currencyformatter.js": {
       "version": "1.0.5",
       "resolved": "https://registry.npmjs.org/currencyformatter.js/-/currencyformatter.js-1.0.5.tgz",
@@ -104616,6 +104635,11 @@
             "@emotion/weak-memoize": "0.2.5"
           }
         },
+        "csstype": {
+          "version": "2.6.21",
+          "resolved": "https://registry.npmjs.org/csstype/-/csstype-2.6.21.tgz",
+          "integrity": "sha512-Z1PhmomIfypOpoMjRQB70jfvy/wxT50qW08YXO5lMIJkrdq4yOTR+AW7FqutScmB9NkLwxo+jU+kZLbofZZq/w=="
+        },
         "dom-helpers": {
           "version": "5.1.4",
           "resolved": "https://registry.npmjs.org/dom-helpers/-/dom-helpers-5.1.4.tgz",
diff --git a/superset-frontend/src/components/Chart/DrillBy/DrillByModal.tsx b/superset-frontend/src/components/Chart/DrillBy/DrillByModal.tsx
index d1e8b39e0c..5053a76711 100644
--- a/superset-frontend/src/components/Chart/DrillBy/DrillByModal.tsx
+++ b/superset-frontend/src/components/Chart/DrillBy/DrillByModal.tsx
@@ -54,11 +54,12 @@ import {
   LOG_ACTIONS_DRILL_BY_MODAL_OPENED,
   LOG_ACTIONS_FURTHER_DRILL_BY,
 } from 'src/logger/LogUtils';
+import { getQuerySettings } from 'src/explore/exploreUtils';
 import { Dataset, DrillByType } from '../types';
 import DrillByChart from './DrillByChart';
 import { ContextMenuItem } from '../ChartContextMenu/ChartContextMenu';
 import { useContextMenu } from '../ChartContextMenu/useContextMenu';
-import { getChartDataRequest } from '../chartAction';
+import { getChartDataRequest, handleChartDataResponse } from '../chartAction';
 import { useDisplayModeToggle } from './useDisplayModeToggle';
 import {
   DrillByBreadcrumb,
@@ -390,13 +391,17 @@ export default function DrillByModal({
 
   useEffect(() => {
     if (drilledFormData) {
+      const [useLegacyApi] = getQuerySettings(drilledFormData);
       setIsChartDataLoading(true);
       setChartDataResult(undefined);
       getChartDataRequest({
         formData: drilledFormData,
       })
-        .then(({ json }) => {
-          setChartDataResult(json.result);
+        .then(({ response, json }) =>
+          handleChartDataResponse(response, json, useLegacyApi),
+        )
+        .then(queriesResponse => {
+          setChartDataResult(queriesResponse);
         })
         .catch(() => {
           addDangerToast(t('Failed to load chart data.'));
diff --git a/superset-frontend/src/components/Chart/chartAction.js b/superset-frontend/src/components/Chart/chartAction.js
index 8cd3785ae5..9e7f961abd 100644
--- a/superset-frontend/src/components/Chart/chartAction.js
+++ b/superset-frontend/src/components/Chart/chartAction.js
@@ -374,6 +374,29 @@ export function addChart(chart, key) {
   return { type: ADD_CHART, chart, key };
 }
 
+export function handleChartDataResponse(response, json, useLegacyApi) {
+  if (isFeatureEnabled(FeatureFlag.GlobalAsyncQueries)) {
+    // deal with getChartDataRequest transforming the response data
+    const result = 'result' in json ? json.result : json;
+    switch (response.status) {
+      case 200:
+        // Query results returned synchronously, meaning query was already cached.
+        return Promise.resolve(result);
+      case 202:
+        // Query is running asynchronously and we must await the results
+        if (useLegacyApi) {
+          return waitForAsyncData(result[0]);
+        }
+        return waitForAsyncData(result);
+      default:
+        throw new Error(
+          `Received unexpected response status (${response.status}) while fetching chart data`,
+        );
+    }
+  }
+  return json.result;
+}
+
 export function exploreJSON(
   formData,
   force = false,
@@ -409,31 +432,11 @@ export function exploreJSON(
 
     dispatch(chartUpdateStarted(controller, formData, key));
 
+    const [useLegacyApi] = getQuerySettings(formData);
     const chartDataRequestCaught = chartDataRequest
-      .then(({ response, json }) => {
-        if (isFeatureEnabled(FeatureFlag.GLOBAL_ASYNC_QUERIES)) {
-          // deal with getChartDataRequest transforming the response data
-          const result = 'result' in json ? json.result : json;
-          const [useLegacyApi] = getQuerySettings(formData);
-          switch (response.status) {
-            case 200:
-              // Query results returned synchronously, meaning query was already cached.
-              return Promise.resolve(result);
-            case 202:
-              // Query is running asynchronously and we must await the results
-              if (useLegacyApi) {
-                return waitForAsyncData(result[0]);
-              }
-              return waitForAsyncData(result);
-            default:
-              throw new Error(
-                `Received unexpected response status (${response.status}) while fetching chart data`,
-              );
-          }
-        }
-
-        return json.result;
-      })
+      .then(({ response, json }) =>
+        handleChartDataResponse(response, json, useLegacyApi),
+      )
       .then(queriesResponse => {
         queriesResponse.forEach(resultItem =>
           dispatch(
@@ -492,7 +495,6 @@ export function exploreJSON(
       });
 
     // only retrieve annotations when calling the legacy API
-    const [useLegacyApi] = getQuerySettings(formData);
     const annotationLayers = useLegacyApi
       ? formData.annotation_layers || []
       : [];
diff --git a/superset-frontend/src/components/Chart/chartActions.test.js b/superset-frontend/src/components/Chart/chartActions.test.js
index b3a6fed9f5..c2a58e6094 100644
--- a/superset-frontend/src/components/Chart/chartActions.test.js
+++ b/superset-frontend/src/components/Chart/chartActions.test.js
@@ -21,10 +21,12 @@ import fetchMock from 'fetch-mock';
 import sinon from 'sinon';
 
 import * as chartlib from '@superset-ui/core';
-import { SupersetClient } from '@superset-ui/core';
+import { FeatureFlag, SupersetClient } from '@superset-ui/core';
 import { LOG_EVENT } from 'src/logger/actions';
 import * as exploreUtils from 'src/explore/exploreUtils';
 import * as actions from 'src/components/Chart/chartAction';
+import * as asyncEvent from 'src/middleware/asyncEvent';
+import { handleChartDataResponse } from 'src/components/Chart/chartAction';
 
 describe('chart actions', () => {
   const MOCK_URL = '/mockURL';
@@ -33,6 +35,7 @@ describe('chart actions', () => {
   let getChartDataUriStub;
   let metadataRegistryStub;
   let buildQueryRegistryStub;
+  let waitForAsyncDataStub;
   let fakeMetadata;
 
   const setupDefaultFetchMock = () => {
@@ -66,6 +69,9 @@ describe('chart actions', () => {
           result_format: 'json',
         }),
       }));
+    waitForAsyncDataStub = sinon
+      .stub(asyncEvent, 'waitForAsyncData')
+      .callsFake(data => Promise.resolve(data));
   });
 
   afterEach(() => {
@@ -74,6 +80,11 @@ describe('chart actions', () => {
     fetchMock.resetHistory();
     metadataRegistryStub.restore();
     buildQueryRegistryStub.restore();
+    waitForAsyncDataStub.restore();
+
+    global.featureFlags = {
+      [FeatureFlag.GlobalAsyncQueries]: false,
+    };
   });
 
   describe('v1 API', () => {
@@ -114,6 +125,36 @@ describe('chart actions', () => {
       expect(fetchMock.calls(mockBigIntUrl)).toHaveLength(1);
       expect(json.value.toString()).toEqual(expectedBigNumber);
     });
+
+    it('handleChartDataResponse should return result if GlobalAsyncQueries flag is disabled', async () => {
+      const result = await handleChartDataResponse(
+        { status: 200 },
+        { result: [1, 2, 3] },
+      );
+      expect(result).toEqual([1, 2, 3]);
+    });
+
+    it('handleChartDataResponse should handle responses when GlobalAsyncQueries flag is enabled and results are returned synchronously', async () => {
+      global.featureFlags = {
+        [FeatureFlag.GlobalAsyncQueries]: true,
+      };
+      const result = await handleChartDataResponse(
+        { status: 200 },
+        { result: [1, 2, 3] },
+      );
+      expect(result).toEqual([1, 2, 3]);
+    });
+
+    it('handleChartDataResponse should handle responses when GlobalAsyncQueries flag is enabled and query is running asynchronously', async () => {
+      global.featureFlags = {
+        [FeatureFlag.GlobalAsyncQueries]: true,
+      };
+      const result = await handleChartDataResponse(
+        { status: 202 },
+        { result: [1, 2, 3] },
+      );
+      expect(result).toEqual([1, 2, 3]);
+    });
   });
 
   describe('legacy API', () => {
diff --git a/superset/security/manager.py b/superset/security/manager.py
index 1cc289f877..e6eb77e645 100644
--- a/superset/security/manager.py
+++ b/superset/security/manager.py
@@ -453,7 +453,7 @@ class SupersetSecurityManager(  # pylint: disable=too-many-public-methods
             level=ErrorLevel.ERROR,
         )
 
-    def get_chart_access_error_object(  # pylint: disable=invalid-name
+    def get_chart_access_error_object(
         self,
         dashboard: "Dashboard",  # pylint: disable=unused-argument
     ) -> SupersetError:
@@ -2088,7 +2088,9 @@ class SupersetSecurityManager(  # pylint: disable=too-many-public-methods
 
             raise SupersetSecurityException(self.get_chart_access_error_object(chart))
 
-    def get_user_by_username(self, username: str, session: Session = None) -> Optional[User]:
+    def get_user_by_username(
+        self, username: str, session: Session = None
+    ) -> Optional[User]:
         """
         Retrieves a user by it's username case sensitive. Optional session parameter
         utility method normally useful for celery tasks where the session