You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by gr...@apache.org on 2019/05/28 22:34:37 UTC

[incubator-superset] branch gg-Test2 created (now 8f5739b)

This is an automated email from the ASF dual-hosted git repository.

graceguo pushed a change to branch gg-Test2
in repository https://gitbox.apache.org/repos/asf/incubator-superset.git.


      at 8f5739b  [SQL Lab] Show warning when user used up localStorage

This branch includes the following new commits:

     new 579040c  [SQL Lab] fix unnecessary offline action
     new 8f5739b  [SQL Lab] Show warning when user used up localStorage

The 2 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.



[incubator-superset] 01/02: [SQL Lab] fix unnecessary offline action

Posted by gr...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

graceguo pushed a commit to branch gg-Test2
in repository https://gitbox.apache.org/repos/asf/incubator-superset.git

commit 579040c65558bda0619deccb0faca31e51c15007
Author: Grace Guo <gr...@airbnb.com>
AuthorDate: Fri May 24 12:16:59 2019 -0700

    [SQL Lab] fix unnecessary offline action
---
 .../javascripts/sqllab/QueryAutoRefresh_spec.jsx   | 72 ++++++++++++++++++++++
 .../assets/spec/javascripts/sqllab/fixtures.js     |  2 +
 .../src/SqlLab/components/QueryAutoRefresh.jsx     | 23 +++++--
 3 files changed, 92 insertions(+), 5 deletions(-)

diff --git a/superset/assets/spec/javascripts/sqllab/QueryAutoRefresh_spec.jsx b/superset/assets/spec/javascripts/sqllab/QueryAutoRefresh_spec.jsx
new file mode 100644
index 0000000..527ecd6
--- /dev/null
+++ b/superset/assets/spec/javascripts/sqllab/QueryAutoRefresh_spec.jsx
@@ -0,0 +1,72 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import React from 'react';
+import { shallow } from 'enzyme';
+import sinon from 'sinon';
+import thunk from 'redux-thunk';
+import configureStore from 'redux-mock-store';
+
+import QueryAutoRefresh from '../../../src/SqlLab/components/QueryAutoRefresh';
+import { initialState, runningQuery } from './fixtures';
+
+describe('QueryAutoRefresh', () => {
+  const middlewares = [thunk];
+  const mockStore = configureStore(middlewares);
+  const sqlLab = {
+    ...initialState.sqlLab,
+    queries: {
+      ryhMUZCGb: runningQuery,
+    },
+  };
+  const state = {
+    ...initialState,
+    sqlLab,
+
+  };
+  const store = mockStore(state);
+
+  const getWrapper = () => (
+    shallow(<QueryAutoRefresh />, {
+      context: { store },
+    }).dive());
+
+  let wrapper;
+
+  it('shouldCheckForQueries', () => {
+    wrapper = getWrapper();
+    expect(wrapper.instance().shouldCheckForQueries()).toBe(true);
+  });
+
+  it('setUserOffline', () => {
+    wrapper = getWrapper();
+    const spy = sinon.spy(wrapper.instance().props.actions, 'setUserOffline');
+
+    // state not changed
+    wrapper.setState({
+      offline: false,
+    });
+    expect(spy.called).toBe(false);
+
+    // state is changed
+    wrapper.setState({
+      offline: true,
+    });
+    expect(spy.callCount).toBe(1);
+  });
+});
diff --git a/superset/assets/spec/javascripts/sqllab/fixtures.js b/superset/assets/spec/javascripts/sqllab/fixtures.js
index f43f43f..6bd090e 100644
--- a/superset/assets/spec/javascripts/sqllab/fixtures.js
+++ b/superset/assets/spec/javascripts/sqllab/fixtures.js
@@ -366,11 +366,13 @@ export const runningQuery = {
   id: 'ryhMUZCGb',
   progress: 90,
   state: 'running',
+  startDttm: Date.now() - 500,
 };
 export const cachedQuery = Object.assign({}, queries[0], { cached: true });
 
 export const initialState = {
   sqlLab: {
+    offline: false,
     alerts: [],
     queries: {},
     databases: {},
diff --git a/superset/assets/src/SqlLab/components/QueryAutoRefresh.jsx b/superset/assets/src/SqlLab/components/QueryAutoRefresh.jsx
index 6704d39..3fcab31 100644
--- a/superset/assets/src/SqlLab/components/QueryAutoRefresh.jsx
+++ b/superset/assets/src/SqlLab/components/QueryAutoRefresh.jsx
@@ -30,9 +30,20 @@ const MAX_QUERY_AGE_TO_POLL = 21600000;
 const QUERY_TIMEOUT_LIMIT = 10000;
 
 class QueryAutoRefresh extends React.PureComponent {
+  constructor(props) {
+    super(props);
+    this.state = {
+      offline: props.offline,
+    };
+  }
   componentWillMount() {
     this.startTimer();
   }
+  componentDidUpdate(prevProps) {
+    if (prevProps.offline !== this.state.offline) {
+      this.props.actions.setUserOffline(this.state.offline);
+    }
+  }
   componentWillUnmount() {
     this.stopTimer();
   }
@@ -70,12 +81,12 @@ class QueryAutoRefresh extends React.PureComponent {
         if (Object.keys(json).length > 0) {
           this.props.actions.refreshQueries(json);
         }
-        this.props.actions.setUserOffline(false);
-        }).catch(() => {
-          this.props.actions.setUserOffline(true);
-        });
+        this.setState({ offline: false });
+      }).catch(() => {
+        this.setState({ offline: true });
+      });
     } else {
-      this.props.actions.setUserOffline(false);
+      this.setState({ offline: false });
     }
   }
   render() {
@@ -83,6 +94,7 @@ class QueryAutoRefresh extends React.PureComponent {
   }
 }
 QueryAutoRefresh.propTypes = {
+  offline: PropTypes.bool.isRequired,
   queries: PropTypes.object.isRequired,
   actions: PropTypes.object.isRequired,
   queriesLastUpdate: PropTypes.number.isRequired,
@@ -90,6 +102,7 @@ QueryAutoRefresh.propTypes = {
 
 function mapStateToProps({ sqlLab }) {
   return {
+    offline: sqlLab.offline,
     queries: sqlLab.queries,
     queriesLastUpdate: sqlLab.queriesLastUpdate,
   };


[incubator-superset] 02/02: [SQL Lab] Show warning when user used up localStorage

Posted by gr...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

graceguo pushed a commit to branch gg-Test2
in repository https://gitbox.apache.org/repos/asf/incubator-superset.git

commit 8f5739bc20bf47097d6a6fb206f33ce40a2435ec
Author: Grace Guo <gr...@airbnb.com>
AuthorDate: Tue May 28 15:31:20 2019 -0700

    [SQL Lab] Show warning when user used up localStorage
---
 .../javascripts/sqllab/utils/emptyQueryResults.js} | 18 ++++++-------
 superset/assets/src/SqlLab/App.jsx                 | 14 +++++++++-
 superset/assets/src/SqlLab/components/App.jsx      | 31 +++++++++++++++++++++-
 .../assets/src/SqlLab/components/SouthPane.jsx     |  5 ++--
 superset/assets/src/SqlLab/constants.js            |  3 +++
 .../assets/src/SqlLab/reducers/getInitialState.js  |  1 +
 superset/assets/src/SqlLab/reducers/index.js       |  2 ++
 .../reducers/{index.js => localStorageUsage.js}    | 14 +++-------
 .../index.js => utils/emptyQueryResults.js}        | 25 ++++++++++-------
 9 files changed, 80 insertions(+), 33 deletions(-)

diff --git a/superset/assets/src/SqlLab/reducers/index.js b/superset/assets/spec/javascripts/sqllab/utils/emptyQueryResults.js
similarity index 71%
copy from superset/assets/src/SqlLab/reducers/index.js
copy to superset/assets/spec/javascripts/sqllab/utils/emptyQueryResults.js
index 9bea4f1..0606b40 100644
--- a/superset/assets/src/SqlLab/reducers/index.js
+++ b/superset/assets/spec/javascripts/sqllab/utils/emptyQueryResults.js
@@ -16,14 +16,14 @@
  * specific language governing permissions and limitations
  * under the License.
  */
-import { combineReducers } from 'redux';
+import emptyQueryResults from '../../../../src/SqlLab/utils/emptyQueryResults';
+import { queries } from '../fixtures';
 
-import sqlLab from './sqlLab';
-import messageToasts from '../../messageToasts/reducers/index';
-import common from './common';
+describe('emptyQueryResults', () => {
+  it('should empty query.results if query.startDttm is > LOCALSTORAGE_MAX_QUERY_AGE', () => {
+    // sample object contains old query
+    const queries = {
 
-export default combineReducers({
-  sqlLab,
-  messageToasts,
-  common,
-});
+    }
+  });
+});
\ No newline at end of file
diff --git a/superset/assets/src/SqlLab/App.jsx b/superset/assets/src/SqlLab/App.jsx
index 359cd73..24bfaa8 100644
--- a/superset/assets/src/SqlLab/App.jsx
+++ b/superset/assets/src/SqlLab/App.jsx
@@ -27,6 +27,7 @@ import getInitialState from './reducers/getInitialState';
 import rootReducer from './reducers/index';
 import { initEnhancer } from '../reduxUtils';
 import App from './components/App';
+import emptyQueryResults from './utils/emptyQueryResults';
 import setupApp from '../setup/setupApp';
 
 import './main.less';
@@ -50,8 +51,19 @@ const sqlLabPersistStateConfig = {
         // it caused configurations passed from server-side got override.
         // see PR 6257 for details
         delete state[path].common; // eslint-disable-line no-param-reassign
-        subset[path] = state[path];
+
+        subset[path] = {
+          ...state[path],
+          queries: emptyQueryResults(state[path].queries),
+        };
       });
+
+      const data = localStorage.getItem('redux') || '';
+      const currentSize = Math.round((data.length * 16) / (8 * 1024) * 100) / 100;
+      if (state.localStorageUsage !== currentSize) {
+        state.localStorageUsage = currentSize; // eslint-disable-line no-param-reassign
+      }
+
       return subset;
     },
   },
diff --git a/superset/assets/src/SqlLab/components/App.jsx b/superset/assets/src/SqlLab/components/App.jsx
index e4e516a..ad1f266 100644
--- a/superset/assets/src/SqlLab/components/App.jsx
+++ b/superset/assets/src/SqlLab/components/App.jsx
@@ -21,6 +21,8 @@ import PropTypes from 'prop-types';
 import { bindActionCreators } from 'redux';
 import { connect } from 'react-redux';
 import $ from 'jquery';
+import { t } from '@superset-ui/translation';
+import debounce from 'lodash/debounce';
 
 import TabbedSqlEditors from './TabbedSqlEditors';
 import QueryAutoRefresh from './QueryAutoRefresh';
@@ -28,6 +30,9 @@ import QuerySearch from './QuerySearch';
 import ToastPresenter from '../../messageToasts/containers/ToastPresenter';
 import * as Actions from '../actions/sqlLab';
 
+// Upper limits for localStorage usage.
+const SQLLAB_LOCALSTORAGE_MAX = 5120;
+
 class App extends React.PureComponent {
   constructor(props) {
     super(props);
@@ -35,6 +40,8 @@ class App extends React.PureComponent {
       hash: window.location.hash,
       contentHeight: '0px',
     };
+
+    this.showLocalStorageUsageWarning = debounce(this.showLocalStorageUsageWarning, 2000);
   }
   componentDidMount() {
     /* eslint-disable react/no-did-mount-set-state */
@@ -42,6 +49,11 @@ class App extends React.PureComponent {
     window.addEventListener('hashchange', this.onHashChanged.bind(this));
     window.addEventListener('resize', this.handleResize.bind(this));
   }
+  componentDidUpdate() {
+    if (this.props.localStorageUsage >= 0.9 * SQLLAB_LOCALSTORAGE_MAX) {
+      this.showLocalStorageUsageWarning(this.props.localStorageUsage);
+    }
+  }
   componentWillUnmount() {
     window.removeEventListener('hashchange', this.onHashChanged.bind(this));
     window.removeEventListener('resize', this.handleResize.bind(this));
@@ -65,6 +77,15 @@ class App extends React.PureComponent {
     const alertHeight = alertEl.length > 0 ? alertEl.outerHeight() : 0;
     return `${window.innerHeight - headerHeight - tabsHeight - warningHeight - alertHeight}px`;
   }
+  showLocalStorageUsageWarning(currentUsage) {
+    this.props.actions.addDangerToast(
+      t('SQL Lab uses your browser\'s local storage to store queries and results.' +
+        `\n Currently, you are using ${currentUsage.toFixed(2)} KB out of ${SQLLAB_LOCALSTORAGE_MAX} KB. storage space.` +
+        '\n To keep SQL Lab from crashing, please delete some query tabs.' +
+        '\n You can re-access these queries by using the Save feature before you delete the tab.' +
+        '\n Note that you will need to close other SQL Lab windows before you do this.'),
+    );
+  }
   handleResize() {
     this.setState({ contentHeight: this.getHeight() });
   }
@@ -91,8 +112,16 @@ class App extends React.PureComponent {
 
 App.propTypes = {
   actions: PropTypes.object,
+  localStorageUsage: PropTypes.number.isRequired,
 };
 
+function mapStateToProps(state) {
+  const { localStorageUsage } = state;
+  return {
+    localStorageUsage,
+  };
+}
+
 function mapDispatchToProps(dispatch) {
   return {
     actions: bindActionCreators(Actions, dispatch),
@@ -101,6 +130,6 @@ function mapDispatchToProps(dispatch) {
 
 export { App };
 export default connect(
-  null,
+  mapStateToProps,
   mapDispatchToProps,
 )(App);
diff --git a/superset/assets/src/SqlLab/components/SouthPane.jsx b/superset/assets/src/SqlLab/components/SouthPane.jsx
index c3dd57b..60d670c 100644
--- a/superset/assets/src/SqlLab/components/SouthPane.jsx
+++ b/superset/assets/src/SqlLab/components/SouthPane.jsx
@@ -27,7 +27,7 @@ import { t } from '@superset-ui/translation';
 import * as Actions from '../actions/sqlLab';
 import QueryHistory from './QueryHistory';
 import ResultSet from './ResultSet';
-import { STATUS_OPTIONS, STATE_BSSTYLE_MAP } from '../constants';
+import { STATUS_OPTIONS, STATE_BSSTYLE_MAP, LOCALSTORAGE_MAX_QUERY_AGE } from '../constants';
 
 const TAB_HEIGHT = 44;
 
@@ -85,7 +85,8 @@ export class SouthPane extends React.PureComponent {
       latestQuery = props.editorQueries[props.editorQueries.length - 1];
     }
     let results;
-    if (latestQuery) {
+    if (latestQuery &&
+      (Date.now() - latestQuery.startDttm) <= LOCALSTORAGE_MAX_QUERY_AGE) {
       results = (
         <ResultSet
           showControls
diff --git a/superset/assets/src/SqlLab/constants.js b/superset/assets/src/SqlLab/constants.js
index 3bf8ce0..8bbec6f 100644
--- a/superset/assets/src/SqlLab/constants.js
+++ b/superset/assets/src/SqlLab/constants.js
@@ -43,3 +43,6 @@ export const TIME_OPTIONS = [
   '90 days ago',
   '1 year ago',
 ];
+
+// 24 hours
+export const LOCALSTORAGE_MAX_QUERY_AGE = 24 * 60 * 60 * 1000;
diff --git a/superset/assets/src/SqlLab/reducers/getInitialState.js b/superset/assets/src/SqlLab/reducers/getInitialState.js
index 535cb3d..2d78154 100644
--- a/superset/assets/src/SqlLab/reducers/getInitialState.js
+++ b/superset/assets/src/SqlLab/reducers/getInitialState.js
@@ -52,6 +52,7 @@ export default function getInitialState({ defaultDbId, ...restBootstrapData }) {
     messageToasts: getToastsFromPyFlashMessages(
       (restBootstrapData.common || {}).flash_messages || [],
     ),
+    localStorageUsage: 0,
     common: {
       flash_messages: restBootstrapData.common.flash_messages,
       conf: restBootstrapData.common.conf,
diff --git a/superset/assets/src/SqlLab/reducers/index.js b/superset/assets/src/SqlLab/reducers/index.js
index 9bea4f1..9d6fd60 100644
--- a/superset/assets/src/SqlLab/reducers/index.js
+++ b/superset/assets/src/SqlLab/reducers/index.js
@@ -19,11 +19,13 @@
 import { combineReducers } from 'redux';
 
 import sqlLab from './sqlLab';
+import localStorageUsage from './localStorageUsage';
 import messageToasts from '../../messageToasts/reducers/index';
 import common from './common';
 
 export default combineReducers({
   sqlLab,
+  localStorageUsage,
   messageToasts,
   common,
 });
diff --git a/superset/assets/src/SqlLab/reducers/index.js b/superset/assets/src/SqlLab/reducers/localStorageUsage.js
similarity index 76%
copy from superset/assets/src/SqlLab/reducers/index.js
copy to superset/assets/src/SqlLab/reducers/localStorageUsage.js
index 9bea4f1..eafbb07 100644
--- a/superset/assets/src/SqlLab/reducers/index.js
+++ b/superset/assets/src/SqlLab/reducers/localStorageUsage.js
@@ -16,14 +16,6 @@
  * specific language governing permissions and limitations
  * under the License.
  */
-import { combineReducers } from 'redux';
-
-import sqlLab from './sqlLab';
-import messageToasts from '../../messageToasts/reducers/index';
-import common from './common';
-
-export default combineReducers({
-  sqlLab,
-  messageToasts,
-  common,
-});
+export default function localStorageUsageReducer(state = 0) {
+  return state;
+}
diff --git a/superset/assets/src/SqlLab/reducers/index.js b/superset/assets/src/SqlLab/utils/emptyQueryResults.js
similarity index 63%
copy from superset/assets/src/SqlLab/reducers/index.js
copy to superset/assets/src/SqlLab/utils/emptyQueryResults.js
index 9bea4f1..dfe365a 100644
--- a/superset/assets/src/SqlLab/reducers/index.js
+++ b/superset/assets/src/SqlLab/utils/emptyQueryResults.js
@@ -16,14 +16,21 @@
  * specific language governing permissions and limitations
  * under the License.
  */
-import { combineReducers } from 'redux';
+import { LOCALSTORAGE_MAX_QUERY_AGE } from '../constants';
 
-import sqlLab from './sqlLab';
-import messageToasts from '../../messageToasts/reducers/index';
-import common from './common';
+export default function emptyQueryResults(queries) {
+  return Object.keys(queries)
+    .reduce((updatedQueries, key) => {
+      const { startDttm } = queries[key];
+      const query = {
+        ...queries[key],
+        results: Date.now() - startDttm > LOCALSTORAGE_MAX_QUERY_AGE ?
+          {} : queries[key].results,
+      };
 
-export default combineReducers({
-  sqlLab,
-  messageToasts,
-  common,
-});
+      return {
+        ...updatedQueries,
+        [key]: query,
+      };
+    }, {});
+}
\ No newline at end of file