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/04/16 11:25:42 UTC
(superset) 06/07: fix: bump client side chart timeouts to use the SUPERSET_WEBSERVER_TIMEOUT (#28018)
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 964dc610afd24679b6f12ca747400c813f4d51c7
Author: Elizabeth Thompson <es...@gmail.com>
AuthorDate: Mon Apr 15 13:16:18 2024 -0700
fix: bump client side chart timeouts to use the SUPERSET_WEBSERVER_TIMEOUT (#28018)
(cherry picked from commit 99c414e4dad9ad608d41af04de0c6f7f53758960)
---
superset-frontend/src/SqlLab/fixtures.ts | 1 +
.../src/components/Chart/chartAction.js | 21 ++--
.../src/components/Chart/chartActions.test.js | 117 +++++++++++++++++----
3 files changed, 112 insertions(+), 27 deletions(-)
diff --git a/superset-frontend/src/SqlLab/fixtures.ts b/superset-frontend/src/SqlLab/fixtures.ts
index 7a08c51876..e79a2bd78c 100644
--- a/superset-frontend/src/SqlLab/fixtures.ts
+++ b/superset-frontend/src/SqlLab/fixtures.ts
@@ -679,6 +679,7 @@ export const initialState = {
DISPLAY_MAX_ROW: 100,
SQLALCHEMY_DOCS_URL: 'test_SQLALCHEMY_DOCS_URL',
SQLALCHEMY_DISPLAY_TEXT: 'test_SQLALCHEMY_DISPLAY_TEXT',
+ SUPERSET_WEBSERVER_TIMEOUT: '300',
},
},
};
diff --git a/superset-frontend/src/components/Chart/chartAction.js b/superset-frontend/src/components/Chart/chartAction.js
index 86e4b6b2a0..4f48e294f3 100644
--- a/superset-frontend/src/components/Chart/chartAction.js
+++ b/superset-frontend/src/components/Chart/chartAction.js
@@ -252,17 +252,20 @@ export async function getChartDataRequest({
export function runAnnotationQuery({
annotation,
- timeout = 60,
+ timeout,
formData = null,
key,
isDashboardRequest = false,
force = false,
}) {
return function (dispatch, getState) {
- const sliceKey = key || Object.keys(getState().charts)[0];
+ const { charts, common } = getState();
+ const sliceKey = key || Object.keys(charts)[0];
+ const queryTimeout = timeout || common.conf.SUPERSET_WEBSERVER_TIMEOUT;
+
// make a copy of formData, not modifying original formData
const fd = {
- ...(formData || getState().charts[sliceKey].latestQueryFormData),
+ ...(formData || charts[sliceKey].latestQueryFormData),
};
if (!annotation.sourceType) {
@@ -313,7 +316,7 @@ export function runAnnotationQuery({
return SupersetClient.post({
url,
signal,
- timeout: timeout * 1000,
+ timeout: queryTimeout * 1000,
headers: { 'Content-Type': 'application/json' },
jsonPayload: buildV1ChartDataPayload({
formData: fd,
@@ -400,19 +403,21 @@ export function handleChartDataResponse(response, json, useLegacyApi) {
export function exploreJSON(
formData,
force = false,
- timeout = 60,
+ timeout,
key,
method,
dashboardId,
ownState,
) {
- return async dispatch => {
+ return async (dispatch, getState) => {
const logStart = Logger.getTimestamp();
const controller = new AbortController();
+ const queryTimeout =
+ timeout || getState().common.conf.SUPERSET_WEBSERVER_TIMEOUT;
const requestParams = {
signal: controller.signal,
- timeout: timeout * 1000,
+ timeout: queryTimeout * 1000,
};
if (dashboardId) requestParams.dashboard_id = dashboardId;
@@ -554,7 +559,7 @@ export const POST_CHART_FORM_DATA = 'POST_CHART_FORM_DATA';
export function postChartFormData(
formData,
force = false,
- timeout = 60,
+ timeout,
key,
dashboardId,
ownState,
diff --git a/superset-frontend/src/components/Chart/chartActions.test.js b/superset-frontend/src/components/Chart/chartActions.test.js
index 7babb7bbb2..69584b1e66 100644
--- a/superset-frontend/src/components/Chart/chartActions.test.js
+++ b/superset-frontend/src/components/Chart/chartActions.test.js
@@ -28,6 +28,27 @@ import * as actions from 'src/components/Chart/chartAction';
import * as asyncEvent from 'src/middleware/asyncEvent';
import { handleChartDataResponse } from 'src/components/Chart/chartAction';
+import configureMockStore from 'redux-mock-store';
+import thunk from 'redux-thunk';
+import { initialState } from 'src/SqlLab/fixtures';
+
+const middlewares = [thunk];
+const mockStore = configureMockStore(middlewares);
+
+const mockGetState = () => ({
+ charts: {
+ chartKey: {
+ latestQueryFormData: {
+ time_grain_sqla: 'P1D',
+ granularity_sqla: 'Date',
+ },
+ },
+ },
+ common: {
+ conf: {},
+ },
+});
+
describe('chart actions', () => {
const MOCK_URL = '/mockURL';
let dispatch;
@@ -94,7 +115,7 @@ describe('chart actions', () => {
it('should query with the built query', async () => {
const actionThunk = actions.postChartFormData({}, null);
- await actionThunk(dispatch);
+ await actionThunk(dispatch, mockGetState);
expect(fetchMock.calls(MOCK_URL)).toHaveLength(1);
expect(fetchMock.calls(MOCK_URL)[0][1].body).toBe(
@@ -165,7 +186,7 @@ describe('chart actions', () => {
it('should dispatch CHART_UPDATE_STARTED action before the query', () => {
const actionThunk = actions.postChartFormData({});
- return actionThunk(dispatch).then(() => {
+ return actionThunk(dispatch, mockGetState).then(() => {
// chart update, trigger query, update form data, success
expect(dispatch.callCount).toBe(5);
expect(fetchMock.calls(MOCK_URL)).toHaveLength(1);
@@ -175,7 +196,7 @@ describe('chart actions', () => {
it('should dispatch TRIGGER_QUERY action with the query', () => {
const actionThunk = actions.postChartFormData({});
- return actionThunk(dispatch).then(() => {
+ return actionThunk(dispatch, mockGetState).then(() => {
// chart update, trigger query, update form data, success
expect(dispatch.callCount).toBe(5);
expect(fetchMock.calls(MOCK_URL)).toHaveLength(1);
@@ -185,7 +206,7 @@ describe('chart actions', () => {
it('should dispatch UPDATE_QUERY_FORM_DATA action with the query', () => {
const actionThunk = actions.postChartFormData({});
- return actionThunk(dispatch).then(() => {
+ return actionThunk(dispatch, mockGetState).then(() => {
// chart update, trigger query, update form data, success
expect(dispatch.callCount).toBe(5);
expect(fetchMock.calls(MOCK_URL)).toHaveLength(1);
@@ -195,7 +216,7 @@ describe('chart actions', () => {
it('should dispatch logEvent async action', () => {
const actionThunk = actions.postChartFormData({});
- return actionThunk(dispatch).then(() => {
+ return actionThunk(dispatch, mockGetState).then(() => {
// chart update, trigger query, update form data, success
expect(dispatch.callCount).toBe(5);
expect(fetchMock.calls(MOCK_URL)).toHaveLength(1);
@@ -209,7 +230,7 @@ describe('chart actions', () => {
it('should dispatch CHART_UPDATE_SUCCEEDED action upon success', () => {
const actionThunk = actions.postChartFormData({});
- return actionThunk(dispatch).then(() => {
+ return actionThunk(dispatch, mockGetState).then(() => {
// chart update, trigger query, update form data, success
expect(dispatch.callCount).toBe(5);
expect(fetchMock.calls(MOCK_URL)).toHaveLength(1);
@@ -226,7 +247,7 @@ describe('chart actions', () => {
const timeoutInSec = 1 / 1000;
const actionThunk = actions.postChartFormData({}, false, timeoutInSec);
- return actionThunk(dispatch).then(() => {
+ return actionThunk(dispatch, mockGetState).then(() => {
// chart update, trigger query, update form data, fail
expect(fetchMock.calls(MOCK_URL)).toHaveLength(1);
expect(dispatch.callCount).toBe(5);
@@ -245,7 +266,7 @@ describe('chart actions', () => {
const timeoutInSec = 100; // Set to a time that is longer than the time this will take to fail
const actionThunk = actions.postChartFormData({}, false, timeoutInSec);
- return actionThunk(dispatch).then(() => {
+ return actionThunk(dispatch, mockGetState).then(() => {
// chart update, trigger query, update form data, fail
expect(dispatch.callCount).toBe(5);
const updateFailedAction = dispatch.args[4][0];
@@ -278,17 +299,6 @@ describe('chart actions', () => {
describe('runAnnotationQuery', () => {
const mockDispatch = jest.fn();
- const mockGetState = () => ({
- charts: {
- chartKey: {
- latestQueryFormData: {
- time_grain_sqla: 'P1D',
- granularity_sqla: 'Date',
- },
- },
- },
- });
-
beforeEach(() => {
jest.clearAllMocks();
});
@@ -342,3 +352,72 @@ describe('chart actions', () => {
});
});
});
+
+describe('chart actions timeout', () => {
+ beforeEach(() => {
+ jest.clearAllMocks();
+ });
+
+ it('should use the timeout from arguments when given', () => {
+ const postSpy = jest.spyOn(SupersetClient, 'post');
+ postSpy.mockImplementation(() => Promise.resolve({ json: { result: [] } }));
+ const timeout = 10; // Set the timeout value here
+ const formData = { datasource: 'table__1' }; // Set the formData here
+ const key = 'chartKey'; // Set the chart key here
+
+ const store = mockStore(initialState);
+ store.dispatch(
+ actions.runAnnotationQuery({
+ annotation: {
+ value: 'annotationValue',
+ sourceType: 'Event',
+ overrides: {},
+ },
+ timeout,
+ formData,
+ key,
+ }),
+ );
+
+ const expectedPayload = {
+ url: expect.any(String),
+ signal: expect.any(AbortSignal),
+ timeout: timeout * 1000,
+ headers: { 'Content-Type': 'application/json' },
+ jsonPayload: expect.any(Object),
+ };
+
+ expect(postSpy).toHaveBeenCalledWith(expectedPayload);
+ });
+
+ it('should use the timeout from common.conf when not passed as an argument', () => {
+ const postSpy = jest.spyOn(SupersetClient, 'post');
+ postSpy.mockImplementation(() => Promise.resolve({ json: { result: [] } }));
+ const formData = { datasource: 'table__1' }; // Set the formData here
+ const key = 'chartKey'; // Set the chart key here
+
+ const store = mockStore(initialState);
+ store.dispatch(
+ actions.runAnnotationQuery({
+ annotation: {
+ value: 'annotationValue',
+ sourceType: 'Event',
+ overrides: {},
+ },
+ undefined,
+ formData,
+ key,
+ }),
+ );
+
+ const expectedPayload = {
+ url: expect.any(String),
+ signal: expect.any(AbortSignal),
+ timeout: initialState.common.conf.SUPERSET_WEBSERVER_TIMEOUT * 1000,
+ headers: { 'Content-Type': 'application/json' },
+ jsonPayload: expect.any(Object),
+ };
+
+ expect(postSpy).toHaveBeenCalledWith(expectedPayload);
+ });
+});