Allow user to force refresh metadata (#5933)

* Allow user to force refresh metadata * fix javascript test error * nit * fix styling * allow custom cache timeout configuration on any database * minor improvement * nit * fix test * nit * preserve the old endpoint
apache · Oct 9, 2018 · 712c1aa · 712c1aa
1 parent 1ee08fc
commit 712c1aa
Show file tree

Hide file tree

Showing 8 changed files with 171 additions and 39 deletions.
diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md
@@ -332,7 +332,7 @@ commands are invoked.
 We use [Mocha](https://mochajs.org/), [Chai](http://chaijs.com/) and [Enzyme](http://airbnb.io/enzyme/) to test Javascript. Tests can be run with:
 
 ```bash
-cd superset/assets/javascripts
+cd superset/assets/spec
 npm install
 npm run test
 ```

diff --git a/superset/assets/spec/javascripts/sqllab/SqlEditorLeftBar_spec.jsx b/superset/assets/spec/javascripts/sqllab/SqlEditorLeftBar_spec.jsx
@@ -131,7 +131,7 @@ describe('SqlEditorLeftBar', () => {
         return d.promise();
       });
       wrapper.instance().fetchSchemas(1);
-      expect(ajaxStub.getCall(0).args[0]).to.equal('/superset/schemas/1/');
+      expect(ajaxStub.getCall(0).args[0]).to.equal('/superset/schemas/1/false/');
       expect(wrapper.state().schemaOptions).to.have.length(3);
     });
     it('should handle error', () => {

diff --git a/superset/assets/src/SqlLab/components/SqlEditorLeftBar.jsx b/superset/assets/src/SqlLab/components/SqlEditorLeftBar.jsx
@@ -6,6 +6,7 @@ import createFilterOptions from 'react-select-fast-filter-options';
 
 import TableElement from './TableElement';
 import AsyncSelect from '../../components/AsyncSelect';
+import RefreshLabel from '../../components/RefreshLabel';
 import { t } from '../../locales';
 
 const $ = require('jquery');
@@ -37,7 +38,7 @@ class SqlEditorLeftBar extends React.PureComponent {
     this.fetchSchemas(this.props.queryEditor.dbId);
     this.fetchTables(this.props.queryEditor.dbId, this.props.queryEditor.schema);
   }
-  onDatabaseChange(db) {
+  onDatabaseChange(db, force) {
     const val = db ? db.value : null;
     this.setState({ schemaOptions: [] });
     this.props.actions.queryEditorSetSchema(this.props.queryEditor, null);
@@ -46,7 +47,7 @@ class SqlEditorLeftBar extends React.PureComponent {
       this.setState({ tableOptions: [] });
     } else {
       this.fetchTables(val, this.props.queryEditor.schema);
-      this.fetchSchemas(val);
+      this.fetchSchemas(val, force || false);
     }
   }
   getTableNamesBySubStr(input) {
@@ -114,11 +115,12 @@ class SqlEditorLeftBar extends React.PureComponent {
     this.props.actions.queryEditorSetSchema(this.props.queryEditor, schema);
     this.fetchTables(this.props.queryEditor.dbId, schema);
   }
-  fetchSchemas(dbId) {
+  fetchSchemas(dbId, force) {
     const actualDbId = dbId || this.props.queryEditor.dbId;
+    const forceRefresh = force || false;
     if (actualDbId) {
       this.setState({ schemaLoading: true });
-      const url = `/superset/schemas/${actualDbId}/`;
+      const url = `/superset/schemas/${actualDbId}/${forceRefresh}/`;
       $.get(url).done((data) => {
         const schemaOptions = data.schemas.map(s => ({ value: s, label: s }));
         this.setState({ schemaOptions, schemaLoading: false });
@@ -144,6 +146,7 @@ class SqlEditorLeftBar extends React.PureComponent {
       tableSelectPlaceholder = t('Select table ');
       tableSelectDisabled = true;
     }
+    const database = this.props.database || {};
     return (
       <div className="clearfix sql-toolbar">
         <div>
@@ -172,20 +175,31 @@ class SqlEditorLeftBar extends React.PureComponent {
           />
         </div>
         <div className="m-t-5">
-          <Select
-            name="select-schema"
-            placeholder={t('Select a schema (%s)', this.state.schemaOptions.length)}
-            options={this.state.schemaOptions}
-            value={this.props.queryEditor.schema}
-            valueRenderer={o => (
-              <div>
-                <span className="text-muted">{t('Schema:')}</span> {o.label}
-              </div>
-            )}
-            isLoading={this.state.schemaLoading}
-            autosize={false}
-            onChange={this.changeSchema.bind(this)}
-          />
+          <div className="row">
+            <div className="col-md-11 col-xs-11" style={{ paddingRight: '2px' }}>
+              <Select
+                name="select-schema"
+                placeholder={t('Select a schema (%s)', this.state.schemaOptions.length)}
+                options={this.state.schemaOptions}
+                value={this.props.queryEditor.schema}
+                valueRenderer={o => (
+                  <div>
+                    <span className="text-muted">{t('Schema:')}</span> {o.label}
+                  </div>
+                )}
+                isLoading={this.state.schemaLoading}
+                autosize={false}
+                onChange={this.changeSchema.bind(this)}
+              />
+            </div>
+            <div className="col-md-1 col-xs-1" style={{ paddingTop: '8px', paddingLeft: '0px' }}>
+              <RefreshLabel
+                onClick={this.onDatabaseChange.bind(
+                    this, { value: database.id }, true)}
+                tooltipContent="force refresh schema list"
+              />
+            </div>
+          </div>
         </div>
         <hr />
         <div className="m-t-5">

diff --git a/superset/assets/src/components/RefreshLabel.jsx b/superset/assets/src/components/RefreshLabel.jsx
@@ -0,0 +1,51 @@
+import React from 'react';
+import PropTypes from 'prop-types';
+import { Label } from 'react-bootstrap';
+import TooltipWrapper from './TooltipWrapper';
+
+const propTypes = {
+  onClick: PropTypes.func,
+  className: PropTypes.string,
+  tooltipContent: PropTypes.string.isRequired,
+};
+
+class RefreshLabel extends React.PureComponent {
+  constructor(props) {
+    super(props);
+    this.state = {
+      hovered: false,
+    };
+  }
+
+  mouseOver() {
+    this.setState({ hovered: true });
+  }
+
+  mouseOut() {
+    this.setState({ hovered: false });
+  }
+
+  render() {
+    const labelStyle = this.state.hovered ? 'primary' : 'default';
+    const tooltip = 'Click to ' + this.props.tooltipContent;
+    return (
+      <TooltipWrapper
+        tooltip={tooltip}
+        label="cache-desc"
+      >
+        <Label
+          className={this.props.className}
+          bsStyle={labelStyle}
+          style={{ fontSize: '13px', marginRight: '5px', cursor: 'pointer' }}
+          onClick={this.props.onClick}
+          onMouseOver={this.mouseOver.bind(this)}
+          onMouseOut={this.mouseOut.bind(this)}
+        >
+          <i className="fa fa-refresh" />
+        </Label>
+      </TooltipWrapper>);
+  }
+}
+RefreshLabel.propTypes = propTypes;
+
+export default RefreshLabel;
diff --git a/superset/cache_util.py b/superset/cache_util.py
@@ -7,30 +7,56 @@
 
 from flask import request
 
-from superset import tables_cache
+from superset import cache, tables_cache
 
 
 def view_cache_key(*unused_args, **unused_kwargs):
     args_hash = hash(frozenset(request.args.items()))
     return 'view/{}/{}'.format(request.path, args_hash)
 
 
-def memoized_func(timeout=5 * 60, key=view_cache_key):
+def default_timeout(*unused_args, **unused_kwargs):
+    return 5 * 60
+
+
+def default_enable_cache(*unused_args, **unused_kwargs):
+    return True
+
+
+def memoized_func(timeout=default_timeout,
+                  key=view_cache_key,
+                  enable_cache=default_enable_cache,
+                  use_tables_cache=False):
     """Use this decorator to cache functions that have predefined first arg.
 
+    If enable_cache() is False,
+        the function will never be cached.
+    If enable_cache() is True,
+        cache is adopted and will timeout in timeout() seconds.
+        If force is True, cache will be refreshed.
+
     memoized_func uses simple_cache and stored the data in memory.
     Key is a callable function that takes function arguments and
     returns the caching key.
     """
     def wrap(f):
-        if tables_cache:
+        selected_cache = None
+        if use_tables_cache and tables_cache:
+            selected_cache = tables_cache
+        elif cache:
+            selected_cache = cache
+
+        if selected_cache:
             def wrapped_f(cls, *args, **kwargs):
+                if not enable_cache(*args, **kwargs):
+                    return f(cls, *args, **kwargs)
+
                 cache_key = key(*args, **kwargs)
-                o = tables_cache.get(cache_key)
+                o = selected_cache.get(cache_key)
                 if not kwargs['force'] and o is not None:
                     return o
                 o = f(cls, *args, **kwargs)
-                tables_cache.set(cache_key, o, timeout=timeout)
+                selected_cache.set(cache_key, o, timeout=timeout(*args, **kwargs))
                 return o
         else:
             # noop

diff --git a/superset/db_engine_specs.py b/superset/db_engine_specs.py
@@ -235,7 +235,8 @@ def convert_dttm(cls, target_type, dttm):
     @classmethod
     @cache_util.memoized_func(
         timeout=600,
-        key=lambda *args, **kwargs: 'db:{}:{}'.format(args[0].id, args[1]))
+        key=lambda *args, **kwargs: 'db:{}:{}'.format(args[0].id, args[1]),
+        use_tables_cache=True)
     def fetch_result_sets(cls, db, datasource_type, force=False):
         """Returns the dictionary {schema : [result_set_name]}.
 
@@ -299,7 +300,21 @@ def patch(cls):
         pass
 
     @classmethod
-    def get_schema_names(cls, inspector):
+    @cache_util.memoized_func(
+        enable_cache=lambda *args, **kwargs: kwargs.get('enable_cache', False),
+        timeout=lambda *args, **kwargs: kwargs.get('cache_timeout'),
+        key=lambda *args, **kwargs: 'db:{}:schema_list'.format(kwargs.get('db_id')))
+    def get_schema_names(cls, inspector, db_id,
+                         enable_cache, cache_timeout, force=False):
+        """A function to get all schema names in this db.
+
+        :param inspector: URI string
+        :param db_id: database id
+        :param enable_cache: whether to enable cache for the function
+        :param cache_timeout: timeout settings for cache in second.
+        :param force: force to refresh
+        :return: a list of schema names
+        """
         return inspector.get_schema_names()
 
     @classmethod
@@ -562,7 +577,8 @@ def epoch_to_dttm(cls):
     @classmethod
     @cache_util.memoized_func(
         timeout=600,
-        key=lambda *args, **kwargs: 'db:{}:{}'.format(args[0].id, args[1]))
+        key=lambda *args, **kwargs: 'db:{}:{}'.format(args[0].id, args[1]),
+        use_tables_cache=True)
     def fetch_result_sets(cls, db, datasource_type, force=False):
         schemas = db.inspector.get_schema_names()
         result_sets = {}
@@ -712,7 +728,8 @@ def epoch_to_dttm(cls):
     @classmethod
     @cache_util.memoized_func(
         timeout=600,
-        key=lambda *args, **kwargs: 'db:{}:{}'.format(args[0].id, args[1]))
+        key=lambda *args, **kwargs: 'db:{}:{}'.format(args[0].id, args[1]),
+        use_tables_cache=True)
     def fetch_result_sets(cls, db, datasource_type, force=False):
         """Returns the dictionary {schema : [result_set_name]}.
 
@@ -993,7 +1010,8 @@ def patch(cls):
     @classmethod
     @cache_util.memoized_func(
         timeout=600,
-        key=lambda *args, **kwargs: 'db:{}:{}'.format(args[0].id, args[1]))
+        key=lambda *args, **kwargs: 'db:{}:{}'.format(args[0].id, args[1]),
+        use_tables_cache=True)
     def fetch_result_sets(cls, db, datasource_type, force=False):
         return BaseEngineSpec.fetch_result_sets(
             db, datasource_type, force=force)
@@ -1448,7 +1466,12 @@ def convert_dttm(cls, target_type, dttm):
         return "'{}'".format(dttm.strftime('%Y-%m-%d %H:%M:%S'))
 
     @classmethod
-    def get_schema_names(cls, inspector):
+    @cache_util.memoized_func(
+        enable_cache=lambda *args, **kwargs: kwargs.get('enable_cache', False),
+        timeout=lambda *args, **kwargs: kwargs.get('cache_timeout'),
+        key=lambda *args, **kwargs: 'db:{}:schema_list'.format(kwargs.get('db_id')))
+    def get_schema_names(cls, inspector, db_id,
+                         enable_cache, cache_timeout, force=False):
         schemas = [row[0] for row in inspector.engine.execute('SHOW SCHEMAS')
                    if not row[0].startswith('_')]
         return schemas

diff --git a/superset/models/core.py b/superset/models/core.py
@@ -647,6 +647,7 @@ class Database(Model, AuditMixinNullable, ImportMixin):
     {
         "metadata_params": {},
         "engine_params": {},
+        "metadata_cache_timeout": {},
         "schemas_allowed_for_csv_upload": []
     }
     """))
@@ -870,8 +871,17 @@ def all_view_names(self, schema=None, force=False):
             pass
         return views
 
-    def all_schema_names(self):
-        return sorted(self.db_engine_spec.get_schema_names(self.inspector))
+    def all_schema_names(self, force_refresh=False):
+        extra = self.get_extra()
+        medatada_cache_timeout = extra.get('metadata_cache_timeout', {})
+        schema_cache_timeout = medatada_cache_timeout.get('schema_cache_timeout')
+        enable_cache = 'schema_cache_timeout' in medatada_cache_timeout
+        return sorted(self.db_engine_spec.get_schema_names(
+            inspector=self.inspector,
+            enable_cache=enable_cache,
+            cache_timeout=schema_cache_timeout,
+            db_id=self.id,
+            force=force_refresh))
 
     @property
     def db_engine_spec(self):

diff --git a/superset/views/core.py b/superset/views/core.py
@@ -211,7 +211,12 @@ class DatabaseView(SupersetModelView, DeleteMixin, YamlExportMixin):  # noqa
             'gets unpacked into the [sqlalchemy.MetaData]'
             '(http://docs.sqlalchemy.org/en/rel_1_0/core/metadata.html'
             '#sqlalchemy.schema.MetaData) call.<br/>'
-            '2. The ``schemas_allowed_for_csv_upload`` is a comma separated list '
+            '2. The ``metadata_cache_timeout`` is a cache timeout setting '
+            'in seconds for metadata fetch of this database. Specify it as '
+            '**"metadata_cache_timeout": {"schema_cache_timeout": 600}**. '
+            'If unset, cache will not be enabled for the functionality. '
+            'A timeout of 0 indicates that the cache never expires.<br/>'
+            '3. The ``schemas_allowed_for_csv_upload`` is a comma separated list '
             'of schemas that CSVs are allowed to upload to. '
             'Specify it as **"schemas_allowed": ["public", "csv_upload"]**. '
             'If database flavor does not support schema or any schema is allowed '
@@ -227,7 +232,7 @@ class DatabaseView(SupersetModelView, DeleteMixin, YamlExportMixin):  # noqa
             'all database schemas. For large data warehouse with thousands of '
             'tables, this can be expensive and put strain on the system.'),
         'cache_timeout': _(
-            'Duration (in seconds) of the caching timeout for this database. '
+            'Duration (in seconds) of the caching timeout for charts of this database. '
             'A timeout of 0 indicates that the cache never expires. '
             'Note this defaults to the global timeout if undefined.'),
         'allow_csv_upload': _(
@@ -242,7 +247,7 @@ class DatabaseView(SupersetModelView, DeleteMixin, YamlExportMixin):  # noqa
         'creator': _('Creator'),
         'changed_on_': _('Last Changed'),
         'sqlalchemy_uri': _('SQLAlchemy URI'),
-        'cache_timeout': _('Cache Timeout'),
+        'cache_timeout': _('Chart Cache Timeout'),
         'extra': _('Extra'),
         'allow_run_sync': _('Allow Run Sync'),
         'allow_run_async': _('Allow Run Async'),
@@ -256,7 +261,8 @@ class DatabaseView(SupersetModelView, DeleteMixin, YamlExportMixin):  # noqa
     def pre_add(self, db):
         db.set_sqlalchemy_uri(db.sqlalchemy_uri)
         security_manager.merge_perm('database_access', db.perm)
-        for schema in db.all_schema_names():
+        # adding a new database we always want to force refresh schema list
+        for schema in db.all_schema_names(force_refresh=True):
             security_manager.merge_perm(
                 'schema_access', security_manager.get_schema_perm(db, schema))
 
@@ -1531,15 +1537,17 @@ def checkbox(self, model_view, id_, attr, value):
     @api
     @has_access_api
     @expose('/schemas/<db_id>/')
-    def schemas(self, db_id):
+    @expose('/schemas/<db_id>/<force_refresh>/')
+    def schemas(self, db_id, force_refresh='true'):
         db_id = int(db_id)
+        force_refresh = force_refresh.lower() == 'true'
         database = (
             db.session
             .query(models.Database)
             .filter_by(id=db_id)
             .one()
         )
-        schemas = database.all_schema_names()
+        schemas = database.all_schema_names(force_refresh=force_refresh)
         schemas = security_manager.schemas_accessible_by_user(database, schemas)
         return Response(
             json.dumps({'schemas': schemas}),