From db052b17ea1782c472687095e1365d071e592dd0 Mon Sep 17 00:00:00 2001 From: Grace Guo Date: Wed, 31 May 2017 15:50:26 -0700 Subject: [PATCH] Add visualize advise for long query (#2879) in SqlLab view, if query takes over 45 seconds, we will show advise to store a summarized data set before user clicks on Visualize button. This advise will not block Visualize button. fixes https://github.com/airbnb/superset/issues/2733 --- .../SqlLab/components/VisualizeModal.jsx | 19 ++++++++++++++++++ .../sqllab/VisualizeModal_spec.jsx | 20 +++++++++++++++++++ 2 files changed, 39 insertions(+) diff --git a/superset/assets/javascripts/SqlLab/components/VisualizeModal.jsx b/superset/assets/javascripts/SqlLab/components/VisualizeModal.jsx index 8fd2ba0dfef..7c19645a68b 100644 --- a/superset/assets/javascripts/SqlLab/components/VisualizeModal.jsx +++ b/superset/assets/javascripts/SqlLab/components/VisualizeModal.jsx @@ -1,4 +1,5 @@ /* global notify */ +import moment from 'moment'; import React from 'react'; import PropTypes from 'prop-types'; import { bindActionCreators } from 'redux'; @@ -11,6 +12,7 @@ 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'; const CHART_TYPES = [ { value: 'dist_bar', label: 'Distribution - Bar Chart', requiresTime: false }, @@ -126,6 +128,22 @@ class VisualizeModal extends React.PureComponent { dbId: this.props.query.dbId, }; } + buildVisualizeAdvise() { + let advise; + const queryDuration = moment.duration(this.props.query.endDttm - this.props.query.startDttm); + if (Math.round(queryDuration.asMilliseconds()) > QUERY_TIMEOUT_THRESHOLD) { + advise = ( + + This query took {Math.round(queryDuration.asSeconds())} seconds to run, + and the explore view times out at {QUERY_TIMEOUT_THRESHOLD / 1000} 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 CREATE TABLE AS feature + to store a summarized data set that you can then explore. + ); + } + return advise; + } visualize() { this.props.actions.createDatasource(this.buildVizOptions(), this) .done(() => { @@ -224,6 +242,7 @@ class VisualizeModal extends React.PureComponent { {alerts} + {this.buildVisualizeAdvise()}
Chart Type diff --git a/superset/assets/spec/javascripts/sqllab/VisualizeModal_spec.jsx b/superset/assets/spec/javascripts/sqllab/VisualizeModal_spec.jsx index aa0be16ec80..b9d36a365d4 100644 --- a/superset/assets/spec/javascripts/sqllab/VisualizeModal_spec.jsx +++ b/superset/assets/spec/javascripts/sqllab/VisualizeModal_spec.jsx @@ -246,6 +246,26 @@ describe('VisualizeModal', () => { }); }); + it('should build visualize advise for long query', () => { + const longQuery = Object.assign({}, queries[0], { endDttm: 1476910666798 }); + const props = { + show: true, + query: longQuery, + }; + const longQueryWrapper = shallow(, { + context: { store }, + }).dive(); + const alertWrapper = shallow(longQueryWrapper.instance().buildVisualizeAdvise()); + expect(alertWrapper.hasClass('alert')).to.equal(true); + expect(alertWrapper.text()).to.contain( + 'This query took 101 seconds to run, and the explore view times out at 45 seconds'); + }); + + it('should not build visualize advise', () => { + const wrapper = getVisualizeModalWrapper(); + expect(wrapper.instance().buildVisualizeAdvise()).to.be.a('undefined'); + }); + describe('visualize', () => { const wrapper = getVisualizeModalWrapper(); const mockOptions = { attr: 'mockOptions' };