Skip to content

Commit

Permalink
use config for front end timeout
Browse files Browse the repository at this point in the history
  • Loading branch information
@eschutho
eschutho committed Apr 12, 2024
1 parent 4a2f84b commit 7d262c8
Show file tree
Hide file tree
Showing 3 changed files with 103 additions and 27 deletions.
1 change: 1 addition & 0 deletions superset-frontend/src/SqlLab/fixtures.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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'

Check failure on line 682 in superset-frontend/src/SqlLab/fixtures.ts

View workflow job for this annotation

GitHub Actions / frontend-build 

Insert `,`
},
},
};
Expand Down
20 changes: 12 additions & 8 deletions superset-frontend/src/components/Chart/chartAction.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -248,17 +248,20 @@ export async function getChartDataRequest({

export function runAnnotationQuery({
annotation,
timeout = 300,
timeout,
formData = null,
key,
isDashboardRequest = false,
force = false,
}) {
return function (dispatch, getState) {
const sliceKey = key || Object.keys(getState().charts)[0];
const {charts, common} = getState()

Check failure on line 258 in superset-frontend/src/components/Chart/chartAction.js

View workflow job for this annotation

GitHub Actions / frontend-build 

Replace `charts,·common}·=·getState()` with `·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) {
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -396,18 +399,19 @@ export function handleChartDataResponse(response, json, useLegacyApi) {
export function exploreJSON(
formData,
force = false,
timeout = 300,
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;

Check failure on line 410 in superset-frontend/src/components/Chart/chartAction.js

View workflow job for this annotation

GitHub Actions / frontend-build 

Insert `⏎·····`

const requestParams = {
signal: controller.signal,
timeout: timeout * 1000,
timeout: queryTimeout * 1000,
};
if (dashboardId) requestParams.dashboard_id = dashboardId;

Expand Down Expand Up @@ -519,7 +523,7 @@ export const POST_CHART_FORM_DATA = 'POST_CHART_FORM_DATA';
export function postChartFormData(
formData,
force = false,
timeout = 300,
timeout,
key,
dashboardId,
ownState,
Expand Down
109 changes: 90 additions & 19 deletions superset-frontend/src/components/Chart/chartActions.test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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: {}

Check failure on line 48 in superset-frontend/src/components/Chart/chartActions.test.js

View workflow job for this annotation

GitHub Actions / frontend-build 

Insert `,`
}

Check failure on line 49 in superset-frontend/src/components/Chart/chartActions.test.js

View workflow job for this annotation

GitHub Actions / frontend-build 

Insert `,`
});

describe('chart actions', () => {
const MOCK_URL = '/mockURL';
let dispatch;
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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);
Expand All @@ -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);
Expand All @@ -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);
Expand All @@ -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);
Expand All @@ -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);
Expand All @@ -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);
Expand All @@ -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];
Expand Down Expand Up @@ -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();
});
Expand Down Expand Up @@ -342,3 +352,64 @@ 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(() =>

Check failure on line 363 in superset-frontend/src/components/Chart/chartActions.test.js

View workflow job for this annotation

GitHub Actions / frontend-build 

Replace `⏎······Promise.resolve({·json:·{·result:·[]·}·}),⏎····` with `·Promise.resolve({·json:·{·result:·[]·}·})`
Promise.resolve({ json: { result: [] } }),
);
const timeout = 10; // Set the timeout value here
const formData = {datasource: 'table__1'}; // Set the formData here

Check failure on line 367 in superset-frontend/src/components/Chart/chartActions.test.js

View workflow job for this annotation

GitHub Actions / frontend-build 

Replace `datasource:·'table__1'` with `·datasource:·'table__1'·`
const key = 'chartKey'; // Set the chart key here

const store = mockStore(initialState);
store.dispatch(actions.runAnnotationQuery({

Check failure on line 371 in superset-frontend/src/components/Chart/chartActions.test.js

View workflow job for this annotation

GitHub Actions / frontend-build 

Insert `⏎······`
annotation: { value: 'annotationValue', sourceType: 'Event', overrides: {} },

Check failure on line 372 in superset-frontend/src/components/Chart/chartActions.test.js

View workflow job for this annotation

GitHub Actions / frontend-build 

Replace `annotation:·{·value:·'annotationValue',·sourceType:·'Event',·overrides:·{}` with `··annotation:·{⏎··········value:·'annotationValue',⏎··········sourceType:·'Event',⏎··········overrides:·{},⏎·······`
timeout,

Check failure on line 373 in superset-frontend/src/components/Chart/chartActions.test.js

View workflow job for this annotation

GitHub Actions / frontend-build 

Insert `··`
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);
});
});

0 comments on commit 7d262c8

Please sign in to comment.