From 4270e89f8cca703af7271a6f825774bd57209c29 Mon Sep 17 00:00:00 2001 From: Grace Guo Date: Sun, 14 May 2017 13:04:32 -0700 Subject: [PATCH 1/2] Show clear and actionable query timeout error message 1. Instead of waiting for a long time or server-side response 504 Gateway timeout, explore view now add a query timeout threshold. After timeout it will show specific querytimeout message. 2. fix alert box close button position. --- superset/assets/javascripts/SqlLab/index.jsx | 4 +-- superset/assets/javascripts/constants.js | 1 + .../javascripts/dashboard/Dashboard.jsx | 2 +- .../explorev2/actions/exploreActions.js | 26 ++++++++++++++----- .../explorev2/components/ChartContainer.jsx | 4 ++- .../assets/javascripts/explorev2/index.jsx | 4 +-- .../explorev2/reducers/exploreReducer.js | 11 ++++++++ superset/assets/javascripts/modules/utils.js | 2 +- .../javascripts/explorev2/actions_spec.js | 21 +++++++++++++++ 9 files changed, 62 insertions(+), 13 deletions(-) create mode 100644 superset/assets/javascripts/constants.js diff --git a/superset/assets/javascripts/SqlLab/index.jsx b/superset/assets/javascripts/SqlLab/index.jsx index 39704a4899f3d..e292c2576c1ae 100644 --- a/superset/assets/javascripts/SqlLab/index.jsx +++ b/superset/assets/javascripts/SqlLab/index.jsx @@ -6,7 +6,7 @@ import thunkMiddleware from 'redux-thunk'; import { getInitialState, sqlLabReducer } from './reducers'; import { initEnhancer } from '../reduxUtils'; -import { initJQueryAjaxCSRF } from '../modules/utils'; +import { initJQueryAjax } from '../modules/utils'; import App from './components/App'; import { appSetup } from '../common'; @@ -15,7 +15,7 @@ import './reactable-pagination.css'; import '../components/FilterableTable/FilterableTableStyles.css'; appSetup(); -initJQueryAjaxCSRF(); +initJQueryAjax(); const appContainer = document.getElementById('app'); const bootstrapData = JSON.parse(appContainer.getAttribute('data-bootstrap')); diff --git a/superset/assets/javascripts/constants.js b/superset/assets/javascripts/constants.js new file mode 100644 index 0000000000000..d82d210601de6 --- /dev/null +++ b/superset/assets/javascripts/constants.js @@ -0,0 +1 @@ +export const QUERY_TIMEOUT_THRESHOLD = 60000; diff --git a/superset/assets/javascripts/dashboard/Dashboard.jsx b/superset/assets/javascripts/dashboard/Dashboard.jsx index 0f586a8b7d998..d451bbb4a96ac 100644 --- a/superset/assets/javascripts/dashboard/Dashboard.jsx +++ b/superset/assets/javascripts/dashboard/Dashboard.jsx @@ -337,7 +337,7 @@ export function dashboardContainer(dashboard, datasources, userid) { $(document).ready(() => { // Getting bootstrapped data from the DOM - utils.initJQueryAjaxCSRF(); + utils.initJQueryAjax(); const dashboardData = $('.dashboard').data('bootstrap'); const state = getInitialState(dashboardData); diff --git a/superset/assets/javascripts/explorev2/actions/exploreActions.js b/superset/assets/javascripts/explorev2/actions/exploreActions.js index 2594a91598cb5..3f683243fb1a0 100644 --- a/superset/assets/javascripts/explorev2/actions/exploreActions.js +++ b/superset/assets/javascripts/explorev2/actions/exploreActions.js @@ -1,5 +1,6 @@ /* eslint camelcase: 0 */ import { getExploreUrl } from '../exploreUtils'; +import { QUERY_TIMEOUT_THRESHOLD } from '../../constants'; const $ = window.$ = require('jquery'); @@ -150,6 +151,11 @@ export function chartUpdateStopped(queryRequest) { return { type: CHART_UPDATE_STOPPED }; } +export const CHART_UPDATE_TIMEOUT = 'CHART_UPDATE_TIMEOUT'; +export function chartUpdateTimeout(statusText) { + return { type: CHART_UPDATE_TIMEOUT, statusText }; +} + export const CHART_UPDATE_FAILED = 'CHART_UPDATE_FAILED'; export function chartUpdateFailed(queryResponse) { return { type: CHART_UPDATE_FAILED, queryResponse }; @@ -234,12 +240,20 @@ export const RUN_QUERY = 'RUN_QUERY'; export function runQuery(formData, force = false) { return function (dispatch) { const url = getExploreUrl(formData, 'json', force); - const queryRequest = $.getJSON(url, function (queryResponse) { - dispatch(chartUpdateSucceeded(queryResponse)); - }).fail(function (err) { - if (err.statusText !== 'abort') { - dispatch(chartUpdateFailed(err.responseJSON)); - } + const queryRequest = $.ajax({ + url, + dataType: 'json', + success(queryResponse) { + dispatch(chartUpdateSucceeded(queryResponse)); + }, + error(err) { + if (err.statusText === 'timeout') { + dispatch(chartUpdateTimeout(err.statusText)); + } else if (err.statusText !== 'abort') { + dispatch(chartUpdateFailed(err.responseJSON)); + } + }, + timeout: QUERY_TIMEOUT_THRESHOLD, }); dispatch(chartUpdateStarted(queryRequest)); }; diff --git a/superset/assets/javascripts/explorev2/components/ChartContainer.jsx b/superset/assets/javascripts/explorev2/components/ChartContainer.jsx index c4f646ae0d020..6c2f0af23ee07 100644 --- a/superset/assets/javascripts/explorev2/components/ChartContainer.jsx +++ b/superset/assets/javascripts/explorev2/components/ChartContainer.jsx @@ -169,12 +169,14 @@ class ChartContainer extends React.PureComponent { renderAlert() { const msg = (
- {this.props.alert} +

); return (
diff --git a/superset/assets/javascripts/explorev2/index.jsx b/superset/assets/javascripts/explorev2/index.jsx index bdf0f24e97e03..39ecab08c57dd 100644 --- a/superset/assets/javascripts/explorev2/index.jsx +++ b/superset/assets/javascripts/explorev2/index.jsx @@ -9,14 +9,14 @@ import { now } from '../modules/dates'; import { initEnhancer } from '../reduxUtils'; import AlertsWrapper from '../components/AlertsWrapper'; import { getControlsState, getFormDataFromControls } from './stores/store'; -import { initJQueryAjaxCSRF } from '../modules/utils'; +import { initJQueryAjax } from '../modules/utils'; import ExploreViewContainer from './components/ExploreViewContainer'; import { exploreReducer } from './reducers/exploreReducer'; import { appSetup } from '../common'; import './main.css'; appSetup(); -initJQueryAjaxCSRF(); +initJQueryAjax(); const exploreViewContainer = document.getElementById('js-explore-view-container'); const bootstrapData = JSON.parse(exploreViewContainer.getAttribute('data-bootstrap')); diff --git a/superset/assets/javascripts/explorev2/reducers/exploreReducer.js b/superset/assets/javascripts/explorev2/reducers/exploreReducer.js index 9957372dd6dbf..f58d21a531fe2 100644 --- a/superset/assets/javascripts/explorev2/reducers/exploreReducer.js +++ b/superset/assets/javascripts/explorev2/reducers/exploreReducer.js @@ -2,6 +2,7 @@ import { getControlsState, getFormDataFromControls } from '../stores/store'; import * as actions from '../actions/exploreActions'; import { now } from '../../modules/dates'; +import { QUERY_TIMEOUT_THRESHOLD } from '../../constants'; export const exploreReducer = function (state, action) { const actionHandlers = { @@ -109,6 +110,16 @@ export const exploreReducer = function (state, action) { triggerQuery: true, }); }, + [actions.CHART_UPDATE_TIMEOUT]() { + return Object.assign({}, state, { + chartStatus: 'failed', + chartAlert: 'Query timeout - visualization query are set to timeout at ' + + `${QUERY_TIMEOUT_THRESHOLD / 1000} seconds. ` + + 'Perhaps your data has grown, your database is under unusual load, ' + + 'or you are simply querying a data source that is to large to be processed within the timeout range. ' + + 'If that is the case, we recommend that you summarize your data further.', + }); + }, [actions.CHART_UPDATE_FAILED]() { return Object.assign({}, state, { chartStatus: 'failed', diff --git a/superset/assets/javascripts/modules/utils.js b/superset/assets/javascripts/modules/utils.js index a8ed50344a684..5cb37ca22aa3f 100644 --- a/superset/assets/javascripts/modules/utils.js +++ b/superset/assets/javascripts/modules/utils.js @@ -189,7 +189,7 @@ export function customizeToolTip(chart, xAxisFormatter, yAxisFormatters) { }); } -export function initJQueryAjaxCSRF() { +export function initJQueryAjax() { // Works in conjunction with a Flask-WTF token as described here: // http://flask-wtf.readthedocs.io/en/stable/csrf.html#javascript-requests const token = $('input#csrf_token').val(); diff --git a/superset/assets/spec/javascripts/explorev2/actions_spec.js b/superset/assets/spec/javascripts/explorev2/actions_spec.js index 239b8d00187bd..8eced723e6a0f 100644 --- a/superset/assets/spec/javascripts/explorev2/actions_spec.js +++ b/superset/assets/spec/javascripts/explorev2/actions_spec.js @@ -1,6 +1,9 @@ import { it, describe } from 'mocha'; import { expect } from 'chai'; +import sinon from 'sinon'; +import $ from 'jquery'; import * as actions from '../../../javascripts/explorev2/actions/exploreActions'; +import * as exploreUtils from '../../../javascripts/explorev2/exploreUtils'; import { defaultState } from '../../../javascripts/explorev2/stores/store'; import { exploreReducer } from '../../../javascripts/explorev2/reducers/exploreReducer'; @@ -16,3 +19,21 @@ describe('reducers', () => { expect(newState.controls.show_legend.value).to.equal(true); }); }); + +describe('runQuery', () => { + it('should handle query timeout', () => { + const dispatch = sinon.spy(); + const urlStub = sinon.stub(exploreUtils, 'getExploreUrl', () => ('mockURL')); + const ajaxStub = sinon.stub($, 'ajax'); + ajaxStub.yieldsTo('error', { statusText: 'timeout' }); + + const request = actions.runQuery({}); + request(dispatch); + + expect(dispatch.callCount).to.equal(2); + expect(dispatch.args[0][0].type).to.equal(actions.CHART_UPDATE_TIMEOUT); + + urlStub.restore(); + ajaxStub.restore(); + }); +}); From dd4b2a5e2603e553c28c2967fb2d37b7d031510a Mon Sep 17 00:00:00 2001 From: Grace Guo Date: Sun, 14 May 2017 13:04:32 -0700 Subject: [PATCH 2/2] Show clear and actionable query timeout error message 1. Instead of waiting for a long time or server-side response 504 Gateway timeout, explore view now add a query timeout threshold. After timeout it will show specific querytimeout message. 2. fix alert box close button position. 3. fix a linting error. --- superset/assets/javascripts/SqlLab/components/SaveQuery.jsx | 1 - 1 file changed, 1 deletion(-) diff --git a/superset/assets/javascripts/SqlLab/components/SaveQuery.jsx b/superset/assets/javascripts/SqlLab/components/SaveQuery.jsx index 9e67cf59416f1..e2b2de5f88a46 100644 --- a/superset/assets/javascripts/SqlLab/components/SaveQuery.jsx +++ b/superset/assets/javascripts/SqlLab/components/SaveQuery.jsx @@ -1,4 +1,3 @@ -/* global notify */ import React from 'react'; import PropTypes from 'prop-types'; import { FormControl, FormGroup, Overlay, Popover, Row, Col } from 'react-bootstrap';