Skip to content

Commit

Permalink
[sql-lab] performance updates - make ui more responsive (#2469)
Browse files Browse the repository at this point in the history
* 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
  • Loading branch information
Alanna Scott committed Mar 27, 2017
1 parent 75e7f2d commit 43dd948
Show file tree
Hide file tree
Showing 7 changed files with 21 additions and 45 deletions.
5 changes: 0 additions & 5 deletions superset/assets/javascripts/SqlLab/actions.js
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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();
Expand Down
36 changes: 20 additions & 16 deletions superset/assets/javascripts/SqlLab/components/QueryAutoRefresh.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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,
};
}
Expand Down
3 changes: 0 additions & 3 deletions superset/assets/javascripts/SqlLab/components/SqlEditor.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -35,7 +34,6 @@ const propTypes = {
};

const defaultProps = {
networkOn: true,
database: null,
latestQuery: null,
hideLeftBar: false,
Expand Down Expand Up @@ -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}
/>
</Col>
Expand Down
Original file line number Diff line number Diff line change
@@ -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';
Expand All @@ -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: {},
};

Expand All @@ -27,7 +25,6 @@ class SqlEditorLeftBar extends React.PureComponent {
schemaOptions: [],
tableLoading: false,
tableOptions: [],
networkOn: true,
};
}
componentWillMount() {
Expand Down Expand Up @@ -125,17 +122,12 @@ class SqlEditorLeftBar extends React.PureComponent {
this.refs[ref].hide();
}
render() {
let networkAlert = null;
if (!this.props.networkOn) {
networkAlert = <p><Label bsStyle="danger">OFFLINE</Label></p>;
}
const shouldShowReset = window.location.search === '?reset=1';
const options = this.state.tableOptions;
const filterOptions = createFilterOptions({ options });
return (
<div className="scrollbar-container">
<div className="clearfix sql-toolbar scrollbar-content">
{networkAlert}
<div>
<AsyncSelect
dataEndpoint="/databaseasync/api/read?_flt_0_expose_in_sqllab=1"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,12 +15,10 @@ const propTypes = {
queryEditors: React.PropTypes.array,
tabHistory: React.PropTypes.array.isRequired,
tables: React.PropTypes.array.isRequired,
networkOn: React.PropTypes.bool,
editorHeight: React.PropTypes.string.isRequired,
};
const defaultProps = {
queryEditors: [],
networkOn: true,
};

let queryCount = 1;
Expand Down Expand Up @@ -199,7 +197,6 @@ class TabbedSqlEditors extends React.PureComponent {
latestQuery={latestQuery}
database={database}
actions={this.props.actions}
networkOn={this.props.networkOn}
hideLeftBar={this.state.hideLeftBar}
/>
}
Expand Down Expand Up @@ -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,
};
Expand Down
7 changes: 0 additions & 7 deletions superset/assets/javascripts/SqlLab/reducers.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ export function getInitialState(defaultDbId) {

return {
alerts: [],
networkOn: true,
queries: {},
databases: {},
queryEditors: [defaultQueryEditor],
Expand Down Expand Up @@ -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.
Expand Down
1 change: 0 additions & 1 deletion superset/assets/spec/javascripts/sqllab/fixtures.js
Original file line number Diff line number Diff line change
Expand Up @@ -250,7 +250,6 @@ export const queries = [

export const initialState = {
alerts: [],
networkOn: true,
queries: {},
databases: {},
queryEditors: [defaultQueryEditor],
Expand Down

0 comments on commit 43dd948

Please sign in to comment.