Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Syncing the timeout param from backend #3329

Merged
merged 2 commits into from
Aug 18, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ import shortid from 'shortid';
import { getExploreUrl } from '../../explore/exploreUtils';
import * as actions from '../actions';
import { VISUALIZE_VALIDATION_ERRORS } from '../constants';
import { QUERY_TIMEOUT_THRESHOLD } from '../../constants';
import visTypes from '../../explore/stores/visTypes';

const CHART_TYPES = Object.keys(visTypes)
Expand All @@ -33,6 +32,7 @@ const propTypes = {
show: PropTypes.bool,
datasource: PropTypes.string,
errorMessage: PropTypes.string,
timeout: PropTypes.number,
};
const defaultProps = {
show: false,
Expand Down Expand Up @@ -133,12 +133,13 @@ class VisualizeModal extends React.PureComponent {
}
buildVisualizeAdvise() {
let advise;
const timeout = this.props.timeout;
const queryDuration = moment.duration(this.props.query.endDttm - this.props.query.startDttm);
if (Math.round(queryDuration.asMilliseconds()) > QUERY_TIMEOUT_THRESHOLD) {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can anyone tell which is the new constant supplanting QUERY_TIMEOUT_THRESHOLD.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SUPERSET_WEBSERVER_TIMEOUT

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @xrmx . why this name got changed ?

if (Math.round(queryDuration.asMilliseconds()) > timeout * 1000) {
advise = (
<Alert bsStyle="warning">
This query took {Math.round(queryDuration.asSeconds())} seconds to run,
and the explore view times out at {QUERY_TIMEOUT_THRESHOLD / 1000} seconds,
and the explore view times out at {timeout} seconds,
following this flow will most likely lead to your query timing out.
We recommend your summarize your data further before following that flow.
If activated you can use the <strong>CREATE TABLE AS</strong> feature
Expand Down Expand Up @@ -291,6 +292,7 @@ function mapStateToProps(state) {
return {
datasource: state.datasource,
errorMessage: state.errorMessage,
timeout: state.common ? state.common.SUPERSET_WEBSERVER_TIMEOUT : null,
};
}

Expand Down
1 change: 0 additions & 1 deletion superset/assets/javascripts/constants.js

This file was deleted.

5 changes: 4 additions & 1 deletion superset/assets/javascripts/dashboard/Dashboard.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,12 @@ import AlertsWrapper from '../components/AlertsWrapper';

import '../../stylesheets/dashboard.css';

const px = require('../modules/superset');
const superset = require('../modules/superset');
const urlLib = require('url');
const utils = require('../modules/utils');

let px;

appSetup();

export function getInitialState(boostrapData) {
Expand Down Expand Up @@ -368,6 +370,7 @@ $(document).ready(() => {
const dashboardData = $('.dashboard').data('bootstrap');

const state = getInitialState(dashboardData);
px = superset(state);
const dashboard = dashboardContainer(state.dashboard, state.datasources, state.user_id);
initDashboardView(dashboard);
dashboard.init();
Expand Down
11 changes: 5 additions & 6 deletions superset/assets/javascripts/explore/actions/chartActions.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import { getExploreUrl } from '../exploreUtils';
import { getFormDataFromControls } from '../stores/store';
import { QUERY_TIMEOUT_THRESHOLD } from '../../constants';
import { triggerQuery } from './exploreActions';

const $ = window.$ = require('jquery');
Expand All @@ -24,8 +23,8 @@ export function chartUpdateStopped(queryRequest) {
}

export const CHART_UPDATE_TIMEOUT = 'CHART_UPDATE_TIMEOUT';
export function chartUpdateTimeout(statusText) {
return { type: CHART_UPDATE_TIMEOUT, statusText };
export function chartUpdateTimeout(statusText, timeout) {
return { type: CHART_UPDATE_TIMEOUT, statusText, timeout };
}

export const CHART_UPDATE_FAILED = 'CHART_UPDATE_FAILED';
Expand All @@ -49,7 +48,7 @@ export function removeChartAlert() {
}

export const RUN_QUERY = 'RUN_QUERY';
export function runQuery(formData, force = false) {
export function runQuery(formData, force = false, timeout = 60) {
return function (dispatch, getState) {
const { explore } = getState();
const lastQueryFormData = getFormDataFromControls(explore.controls);
Expand All @@ -62,12 +61,12 @@ export function runQuery(formData, force = false) {
},
error(err) {
if (err.statusText === 'timeout') {
dispatch(chartUpdateTimeout(err.statusText));
dispatch(chartUpdateTimeout(err.statusText, timeout));
} else if (err.statusText !== 'abort') {
dispatch(chartUpdateFailed(err.responseJSON));
}
},
timeout: QUERY_TIMEOUT_THRESHOLD,
timeout: timeout * 1000,
});
dispatch(chartUpdateStarted(queryRequest, lastQueryFormData));
dispatch(triggerQuery(false));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ const propTypes = {
standalone: PropTypes.bool,
datasourceType: PropTypes.string,
datasourceId: PropTypes.number,
timeout: PropTypes.number,
};

class ChartContainer extends React.PureComponent {
Expand Down Expand Up @@ -148,7 +149,7 @@ class ChartContainer extends React.PureComponent {
}

runQuery() {
this.props.actions.runQuery(this.props.formData, true);
this.props.actions.runQuery(this.props.formData, true, this.props.timeout);
}

updateChartTitle(newTitle) {
Expand Down Expand Up @@ -346,6 +347,7 @@ function mapStateToProps({ explore, chart }) {
chartUpdateStartTime: chart.chartUpdateStartTime,
latestQueryFormData: chart.latestQueryFormData,
queryResponse: chart.queryResponse,
timeout: explore.common.conf.SUPERSET_WEBSERVER_TIMEOUT,
};
}

Expand Down
13 changes: 7 additions & 6 deletions superset/assets/javascripts/explore/reducers/chartReducer.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
/* eslint camelcase: 0 */
import { now } from '../../modules/dates';
import * as actions from '../actions/chartActions';
import { QUERY_TIMEOUT_THRESHOLD } from '../../constants';

export default function chartReducer(state = {}, action) {
const actionHandlers = {
Expand Down Expand Up @@ -41,11 +40,13 @@ export default function chartReducer(state = {}, action) {
[actions.CHART_UPDATE_TIMEOUT]() {
return Object.assign({}, state, {
chartStatus: 'failed',
chartAlert: '<strong>Query timeout</strong> - 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.',
chartAlert: (
'<strong>Query timeout</strong> - visualization query are set to timeout at ' +
`${action.timeout} 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]() {
Expand Down
14 changes: 8 additions & 6 deletions superset/assets/javascripts/modules/superset.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,13 @@ import Mustache from 'mustache';
import vizMap from '../../visualizations/main';
import { getExploreUrl } from '../explore/exploreUtils';
import { applyDefaultFormData } from '../explore/stores/store';
import { QUERY_TIMEOUT_THRESHOLD } from '../constants';

const utils = require('./utils');

/* eslint wrap-iife: 0 */
const px = function () {
const px = function (state) {
let slice;
const timeout = state.common.conf.SUPERSET_WEBSERVER_TIMEOUT;
function getParam(name) {
/* eslint no-useless-escape: 0 */
const formattedName = name.replace(/[\[]/, '\\[').replace(/[\]]/, '\\]');
Expand Down Expand Up @@ -157,8 +157,10 @@ const px = function () {
}
if (xhr) {
if (xhr.statusText === 'timeout') {
errHtml += '<div class="alert alert-warning">' +
`Query timeout - visualization query are set to time out at ${QUERY_TIMEOUT_THRESHOLD / 1000} seconds.</div>`;
errHtml += (
'<div class="alert alert-warning">' +
'Query timeout - visualization query are set to time out ' +
`at ${timeout} seconds.</div>`);
} else {
const extendedMsg = this.getErrorMsg(xhr);
if (extendedMsg) {
Expand Down Expand Up @@ -210,7 +212,7 @@ const px = function () {
container.css('height', this.height());
$.ajax({
url: this.jsonEndpoint(),
timeout: QUERY_TIMEOUT_THRESHOLD,
timeout: timeout * 1000,
success: (queryResponse) => {
try {
vizMap[formData.viz_type](this, queryResponse);
Expand Down Expand Up @@ -251,5 +253,5 @@ const px = function () {
initFavStars,
Slice,
};
}();
};
module.exports = px;
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,10 @@ global.notify = {
describe('VisualizeModal', () => {
const middlewares = [thunk];
const mockStore = configureStore(middlewares);
const initialState = sqlLabReducer(undefined, {});
const initialState = sqlLabReducer({}, {});
initialState.common = {
SUPERSET_WEBSERVER_TIMEOUT: 45,
};
const store = mockStore(initialState);
const mockedProps = {
show: true,
Expand Down