diff --git a/superset-frontend/src/SqlLab/fixtures.ts b/superset-frontend/src/SqlLab/fixtures.ts index 845e2209b5c8d..742145c58ca6d 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 9597a3cbb3c64..eb51021d0c5af 100644 --- a/superset-frontend/src/components/Chart/chartAction.js +++ b/superset-frontend/src/components/Chart/chartAction.js @@ -248,17 +248,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) { @@ -309,7 +312,7 @@ export function runAnnotationQuery({ return SupersetClient.post({ url, signal, - timeout: timeout * 1000, + timeout: queryTimeout * 1000, headers: { 'Content-Type': 'application/json' }, jsonPayload: buildV1ChartDataPayload({ formData: fd, @@ -396,18 +399,20 @@ export function handleChartDataResponse(response, json, useLegacyApi) { export function exploreJSON( formData, force = false, - timeout = 60, + timeout, key, 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; @@ -519,7 +524,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 c2a58e6094404..129c17dbe0b4d 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); + }); +});