From 43dd948476190cf75649164f57a9b73687ca3d99 Mon Sep 17 00:00:00 2001 From: Alanna Scott Date: Mon, 27 Mar 2017 12:46:36 -0700 Subject: [PATCH] [sql-lab] performance updates - make ui more responsive (#2469) * remove network status feature * only fetch queries if there are started or running queries * don't use local storage * remove last network status from actions * don't remove support for local storage * address pr comments and linting * use .some rather than .forEach --- superset/assets/javascripts/SqlLab/actions.js | 5 --- .../SqlLab/components/QueryAutoRefresh.jsx | 36 ++++++++++--------- .../SqlLab/components/SqlEditor.jsx | 3 -- .../SqlLab/components/SqlEditorLeftBar.jsx | 10 +----- .../SqlLab/components/TabbedSqlEditors.jsx | 4 --- .../assets/javascripts/SqlLab/reducers.js | 7 ---- .../spec/javascripts/sqllab/fixtures.js | 1 - 7 files changed, 21 insertions(+), 45 deletions(-) diff --git a/superset/assets/javascripts/SqlLab/actions.js b/superset/assets/javascripts/SqlLab/actions.js index c6d48445fa890..3a3f64e15d31f 100644 --- a/superset/assets/javascripts/SqlLab/actions.js +++ b/superset/assets/javascripts/SqlLab/actions.js @@ -24,7 +24,6 @@ export const SET_ACTIVE_SOUTHPANE_TAB = 'SET_ACTIVE_SOUTHPANE_TAB'; export const ADD_ALERT = 'ADD_ALERT'; export const REMOVE_ALERT = 'REMOVE_ALERT'; export const REFRESH_QUERIES = 'REFRESH_QUERIES'; -export const SET_NETWORK_STATUS = 'SET_NETWORK_STATUS'; export const RUN_QUERY = 'RUN_QUERY'; export const START_QUERY = 'START_QUERY'; export const STOP_QUERY = 'STOP_QUERY'; @@ -173,10 +172,6 @@ export function cloneQueryToNewTab(query) { return { type: CLONE_QUERY_TO_NEW_TAB, query }; } -export function setNetworkStatus(networkOn) { - return { type: SET_NETWORK_STATUS, networkOn }; -} - export function addAlert(alert) { const o = Object.assign({}, alert); o.id = shortid.generate(); diff --git a/superset/assets/javascripts/SqlLab/components/QueryAutoRefresh.jsx b/superset/assets/javascripts/SqlLab/components/QueryAutoRefresh.jsx index 458845dea85d0..b52128898d82c 100644 --- a/superset/assets/javascripts/SqlLab/components/QueryAutoRefresh.jsx +++ b/superset/assets/javascripts/SqlLab/components/QueryAutoRefresh.jsx @@ -14,6 +14,13 @@ class QueryAutoRefresh extends React.PureComponent { componentWillUnmount() { this.stopTimer(); } + shouldCheckForQueries() { + // if there are started or running queries, this method should return true + const { queries } = this.props; + const queryKeys = Object.keys(queries); + const queriesAsArray = queryKeys.map(key => queries[key]); + return queriesAsArray.some(q => q.state === 'running' || q.state === 'started'); + } startTimer() { if (!(this.timer)) { this.timer = setInterval(this.stopwatch.bind(this), QUERY_UPDATE_FREQ); @@ -24,32 +31,29 @@ class QueryAutoRefresh extends React.PureComponent { this.timer = null; } stopwatch() { - const url = '/superset/queries/' + (this.props.queriesLastUpdate - QUERY_UPDATE_BUFFER_MS); - // No updates in case of failure. - $.getJSON(url, (data) => { - if (Object.keys(data).length > 0) { - this.props.actions.refreshQueries(data); - } - this.props.actions.setNetworkStatus(true); - }) - .fail(() => { - this.props.actions.setNetworkStatus(false); - }); + // only poll /superset/queries/ if there are started or running queries + if (this.shouldCheckForQueries()) { + const url = '/superset/queries/' + (this.props.queriesLastUpdate - QUERY_UPDATE_BUFFER_MS); + $.getJSON(url, (data) => { + if (Object.keys(data).length > 0) { + this.props.actions.refreshQueries(data); + } + }); + } } render() { return null; } } QueryAutoRefresh.propTypes = { - actions: React.PropTypes.object, - queriesLastUpdate: React.PropTypes.number, -}; -QueryAutoRefresh.defaultProps = { - // queries: null, + queries: React.PropTypes.object.isRequired, + actions: React.PropTypes.object.isRequired, + queriesLastUpdate: React.PropTypes.number.isRequired, }; function mapStateToProps(state) { return { + queries: state.queries, queriesLastUpdate: state.queriesLastUpdate, }; } diff --git a/superset/assets/javascripts/SqlLab/components/SqlEditor.jsx b/superset/assets/javascripts/SqlLab/components/SqlEditor.jsx index 92bbb426b6bdf..6765d90121010 100644 --- a/superset/assets/javascripts/SqlLab/components/SqlEditor.jsx +++ b/superset/assets/javascripts/SqlLab/components/SqlEditor.jsx @@ -26,7 +26,6 @@ const propTypes = { height: React.PropTypes.string.isRequired, database: React.PropTypes.object, latestQuery: React.PropTypes.object, - networkOn: React.PropTypes.bool, tables: React.PropTypes.array.isRequired, editorQueries: React.PropTypes.array.isRequired, dataPreviewQueries: React.PropTypes.array.isRequired, @@ -35,7 +34,6 @@ const propTypes = { }; const defaultProps = { - networkOn: true, database: null, latestQuery: null, hideLeftBar: false, @@ -190,7 +188,6 @@ class SqlEditor extends React.PureComponent { style={{ height: this.props.height }} queryEditor={this.props.queryEditor} tables={this.props.tables} - networkOn={this.props.networkOn} actions={this.props.actions} /> diff --git a/superset/assets/javascripts/SqlLab/components/SqlEditorLeftBar.jsx b/superset/assets/javascripts/SqlLab/components/SqlEditorLeftBar.jsx index 5a2c2d312ee5e..422c47d7899f8 100644 --- a/superset/assets/javascripts/SqlLab/components/SqlEditorLeftBar.jsx +++ b/superset/assets/javascripts/SqlLab/components/SqlEditorLeftBar.jsx @@ -1,6 +1,6 @@ const $ = window.$ = require('jquery'); import React from 'react'; -import { Label, Button } from 'react-bootstrap'; +import { Button } from 'react-bootstrap'; import TableElement from './TableElement'; import AsyncSelect from '../../components/AsyncSelect'; import Select from 'react-virtualized-select'; @@ -10,12 +10,10 @@ const propTypes = { queryEditor: React.PropTypes.object.isRequired, tables: React.PropTypes.array, actions: React.PropTypes.object, - networkOn: React.PropTypes.bool, }; const defaultProps = { tables: [], - networkOn: true, actions: {}, }; @@ -27,7 +25,6 @@ class SqlEditorLeftBar extends React.PureComponent { schemaOptions: [], tableLoading: false, tableOptions: [], - networkOn: true, }; } componentWillMount() { @@ -125,17 +122,12 @@ class SqlEditorLeftBar extends React.PureComponent { this.refs[ref].hide(); } render() { - let networkAlert = null; - if (!this.props.networkOn) { - networkAlert =

; - } const shouldShowReset = window.location.search === '?reset=1'; const options = this.state.tableOptions; const filterOptions = createFilterOptions({ options }); return (
- {networkAlert}
} @@ -235,7 +232,6 @@ function mapStateToProps(state) { queryEditors: state.queryEditors, queries: state.queries, tabHistory: state.tabHistory, - networkOn: state.networkOn, tables: state.tables, defaultDbId: state.defaultDbId, }; diff --git a/superset/assets/javascripts/SqlLab/reducers.js b/superset/assets/javascripts/SqlLab/reducers.js index e6eb073ce6579..7204a1cfa4eee 100644 --- a/superset/assets/javascripts/SqlLab/reducers.js +++ b/superset/assets/javascripts/SqlLab/reducers.js @@ -17,7 +17,6 @@ export function getInitialState(defaultDbId) { return { alerts: [], - networkOn: true, queries: {}, databases: {}, queryEditors: [defaultQueryEditor], @@ -230,12 +229,6 @@ export const sqlLabReducer = function (state, action) { [actions.REMOVE_ALERT]() { return removeFromArr(state, 'alerts', action.alert); }, - [actions.SET_NETWORK_STATUS]() { - if (state.networkOn !== action.networkOn) { - return Object.assign({}, state, { networkOn: action.networkOn }); - } - return state; - }, [actions.REFRESH_QUERIES]() { let newQueries = Object.assign({}, state.queries); // Fetch the updates to the queries present in the store. diff --git a/superset/assets/spec/javascripts/sqllab/fixtures.js b/superset/assets/spec/javascripts/sqllab/fixtures.js index f9587492f0c86..fb854da67de73 100644 --- a/superset/assets/spec/javascripts/sqllab/fixtures.js +++ b/superset/assets/spec/javascripts/sqllab/fixtures.js @@ -250,7 +250,6 @@ export const queries = [ export const initialState = { alerts: [], - networkOn: true, queries: {}, databases: {}, queryEditors: [defaultQueryEditor],