allow cache and force refresh on table list (#6078)

* allow cache and force refresh on table list

* wording

* flake8

* javascript test

* address comments

* nit
This commit is contained in:
Junda Yang 2018-10-16 13:14:45 -07:00 committed by Beto Dealmeida
parent 7d49255926
commit 177bed3bb6
6 changed files with 113 additions and 80 deletions

View File

@ -41,14 +41,10 @@ describe('SqlEditorLeftBar', () => {
expect(wrapper.find(TableElement)).toHaveLength(1); expect(wrapper.find(TableElement)).toHaveLength(1);
}); });
describe('onDatabaseChange', () => { describe('onDatabaseChange', () => {
it('should fetch tables', () => { it('should fetch schemas', () => {
sinon.stub(wrapper.instance(), 'fetchTables');
sinon.stub(wrapper.instance(), 'fetchSchemas'); sinon.stub(wrapper.instance(), 'fetchSchemas');
wrapper.instance().onDatabaseChange({ value: 1, label: 'main' }); wrapper.instance().onDatabaseChange({ value: 1, label: 'main' });
expect(wrapper.instance().fetchTables.getCall(0).args[0]).toBe(1);
expect(wrapper.instance().fetchSchemas.getCall(0).args[0]).toBe(1); expect(wrapper.instance().fetchSchemas.getCall(0).args[0]).toBe(1);
wrapper.instance().fetchTables.restore();
wrapper.instance().fetchSchemas.restore(); wrapper.instance().fetchSchemas.restore();
}); });
it('should clear tableOptions', () => { it('should clear tableOptions', () => {
@ -103,9 +99,9 @@ describe('SqlEditorLeftBar', () => {
d.resolve(tables); d.resolve(tables);
return d.promise(); return d.promise();
}); });
wrapper.instance().fetchTables(1, 'main', 'birth_names'); wrapper.instance().fetchTables(1, 'main', 'true', 'birth_names');
expect(ajaxStub.getCall(0).args[0]).toBe('/superset/tables/1/main/birth_names/'); expect(ajaxStub.getCall(0).args[0]).toBe('/superset/tables/1/main/birth_names/true/');
expect(wrapper.state().tableLength).toBe(3); expect(wrapper.state().tableLength).toBe(3);
}); });
it('should handle error', () => { it('should handle error', () => {

View File

@ -40,13 +40,10 @@ class SqlEditorLeftBar extends React.PureComponent {
} }
onDatabaseChange(db, force) { onDatabaseChange(db, force) {
const val = db ? db.value : null; const val = db ? db.value : null;
this.setState({ schemaOptions: [] }); this.setState({ schemaOptions: [], tableOptions: [] });
this.props.actions.queryEditorSetSchema(this.props.queryEditor, null); this.props.actions.queryEditorSetSchema(this.props.queryEditor, null);
this.props.actions.queryEditorSetDb(this.props.queryEditor, val); this.props.actions.queryEditorSetDb(this.props.queryEditor, val);
if (!(db)) { if (db) {
this.setState({ tableOptions: [] });
} else {
this.fetchTables(val, this.props.queryEditor.schema);
this.fetchSchemas(val, force || false); this.fetchSchemas(val, force || false);
} }
} }
@ -69,11 +66,12 @@ class SqlEditorLeftBar extends React.PureComponent {
resetState() { resetState() {
this.props.actions.resetState(); this.props.actions.resetState();
} }
fetchTables(dbId, schema, substr) { fetchTables(dbId, schema, force, substr) {
// This can be large so it shouldn't be put in the Redux store // This can be large so it shouldn't be put in the Redux store
const forceRefresh = force || false;
if (dbId && schema) { if (dbId && schema) {
this.setState({ tableLoading: true, tableOptions: [] }); this.setState({ tableLoading: true, tableOptions: [] });
const url = `/superset/tables/${dbId}/${schema}/${substr}/`; const url = `/superset/tables/${dbId}/${schema}/${substr}/${forceRefresh}/`;
$.get(url).done((data) => { $.get(url).done((data) => {
const filterOptions = createFilterOptions({ options: data.options }); const filterOptions = createFilterOptions({ options: data.options });
this.setState({ this.setState({
@ -110,10 +108,10 @@ class SqlEditorLeftBar extends React.PureComponent {
} }
this.props.actions.addTable(this.props.queryEditor, tableName, schemaName); this.props.actions.addTable(this.props.queryEditor, tableName, schemaName);
} }
changeSchema(schemaOpt) { changeSchema(schemaOpt, force) {
const schema = (schemaOpt) ? schemaOpt.value : null; const schema = (schemaOpt) ? schemaOpt.value : null;
this.props.actions.queryEditorSetSchema(this.props.queryEditor, schema); this.props.actions.queryEditorSetSchema(this.props.queryEditor, schema);
this.fetchTables(this.props.queryEditor.dbId, schema); this.fetchTables(this.props.queryEditor.dbId, schema, force);
} }
fetchSchemas(dbId, force) { fetchSchemas(dbId, force) {
const actualDbId = dbId || this.props.queryEditor.dbId; const actualDbId = dbId || this.props.queryEditor.dbId;
@ -196,7 +194,7 @@ class SqlEditorLeftBar extends React.PureComponent {
<RefreshLabel <RefreshLabel
onClick={this.onDatabaseChange.bind( onClick={this.onDatabaseChange.bind(
this, { value: database.id }, true)} this, { value: database.id }, true)}
tooltipContent="force refresh schema list" tooltipContent={t('force refresh schema list')}
/> />
</div> </div>
</div> </div>
@ -214,30 +212,41 @@ class SqlEditorLeftBar extends React.PureComponent {
</i>) </i>)
</small> </small>
</ControlLabel> </ControlLabel>
{this.props.queryEditor.schema && <div className="row">
<Select <div className="col-md-11 col-xs-11" style={{ paddingRight: '2px' }}>
name="select-table" {this.props.queryEditor.schema &&
ref="selectTable" <Select
isLoading={this.state.tableLoading} name="select-table"
placeholder={t('Select table or type table name')} ref="selectTable"
autosize={false} isLoading={this.state.tableLoading}
onChange={this.changeTable.bind(this)} placeholder={t('Select table or type table name')}
filterOptions={this.state.filterOptions} autosize={false}
options={this.state.tableOptions} onChange={this.changeTable.bind(this)}
/> filterOptions={this.state.filterOptions}
} options={this.state.tableOptions}
{!this.props.queryEditor.schema && />
<Select }
async {!this.props.queryEditor.schema &&
name="async-select-table" <Select
ref="selectTable" async
placeholder={tableSelectPlaceholder} name="async-select-table"
disabled={tableSelectDisabled} ref="selectTable"
autosize={false} placeholder={tableSelectPlaceholder}
onChange={this.changeTable.bind(this)} disabled={tableSelectDisabled}
loadOptions={this.getTableNamesBySubStr.bind(this)} autosize={false}
/> onChange={this.changeTable.bind(this)}
} loadOptions={this.getTableNamesBySubStr.bind(this)}
/>
}
</div>
<div className="col-md-1 col-xs-1" style={{ paddingTop: '8px', paddingLeft: '0px' }}>
<RefreshLabel
onClick={this.changeSchema.bind(
this, { value: this.props.queryEditor.schema }, true)}
tooltipContent={t('force refresh table list')}
/>
</div>
</div>
</div> </div>
<hr /> <hr />
<div className="m-t-5"> <div className="m-t-5">

View File

@ -9,25 +9,17 @@ def view_cache_key(*unused_args, **unused_kwargs):
return 'view/{}/{}'.format(request.path, args_hash) return 'view/{}/{}'.format(request.path, args_hash)
def default_timeout(*unused_args, **unused_kwargs): def memoized_func(key=view_cache_key, use_tables_cache=False):
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. """Use this decorator to cache functions that have predefined first arg.
If enable_cache() is False, enable_cache is treated as True by default,
the function will never be cached. except enable_cache = False is passed to the decorated function.
If enable_cache() is True,
cache is adopted and will timeout in timeout() seconds. force means whether to force refresh the cache and is treated as False by default,
If force is True, cache will be refreshed. except force = True is passed to the decorated function.
timeout of cache is set to 600 seconds by default,
except cache_timeout = {timeout in seconds} is passed to the decorated function.
memoized_func uses simple_cache and stored the data in memory. memoized_func uses simple_cache and stored the data in memory.
Key is a callable function that takes function arguments and Key is a callable function that takes function arguments and
@ -42,15 +34,16 @@ def memoized_func(timeout=default_timeout,
if selected_cache: if selected_cache:
def wrapped_f(cls, *args, **kwargs): def wrapped_f(cls, *args, **kwargs):
if not enable_cache(*args, **kwargs): if not kwargs.get('enable_cache', True):
return f(cls, *args, **kwargs) return f(cls, *args, **kwargs)
cache_key = key(*args, **kwargs) cache_key = key(*args, **kwargs)
o = selected_cache.get(cache_key) o = selected_cache.get(cache_key)
if not kwargs['force'] and o is not None: if not kwargs.get('force') and o is not None:
return o return o
o = f(cls, *args, **kwargs) o = f(cls, *args, **kwargs)
selected_cache.set(cache_key, o, timeout=timeout(*args, **kwargs)) selected_cache.set(cache_key, o,
timeout=kwargs.get('cache_timeout', 600))
return o return o
else: else:
# noop # noop

View File

@ -228,7 +228,6 @@ class BaseEngineSpec(object):
@classmethod @classmethod
@cache_util.memoized_func( @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) use_tables_cache=True)
def fetch_result_sets(cls, db, datasource_type, force=False): def fetch_result_sets(cls, db, datasource_type, force=False):
@ -295,8 +294,6 @@ class BaseEngineSpec(object):
@classmethod @classmethod
@cache_util.memoized_func( @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'))) key=lambda *args, **kwargs: 'db:{}:schema_list'.format(kwargs.get('db_id')))
def get_schema_names(cls, inspector, db_id, def get_schema_names(cls, inspector, db_id,
enable_cache, cache_timeout, force=False): enable_cache, cache_timeout, force=False):
@ -312,9 +309,21 @@ class BaseEngineSpec(object):
return inspector.get_schema_names() return inspector.get_schema_names()
@classmethod @classmethod
def get_table_names(cls, schema, inspector): @cache_util.memoized_func(
key=lambda *args, **kwargs: 'db:{db_id}:schema:{schema}:table_list'.format(
db_id=kwargs.get('db_id'), schema=kwargs.get('schema')))
def get_table_names(cls, inspector, db_id, schema,
enable_cache, cache_timeout, force=False):
return sorted(inspector.get_table_names(schema)) return sorted(inspector.get_table_names(schema))
@classmethod
@cache_util.memoized_func(
key=lambda *args, **kwargs: 'db:{db_id}:schema:{schema}:view_list'.format(
db_id=kwargs.get('db_id'), schema=kwargs.get('schema')))
def get_view_names(cls, inspector, db_id, schema,
enable_cache, cache_timeout, force=False):
return sorted(inspector.get_view_names(schema))
@classmethod @classmethod
def where_latest_partition( def where_latest_partition(
cls, table_name, schema, database, qry, columns=None): cls, table_name, schema, database, qry, columns=None):
@ -438,7 +447,11 @@ class PostgresEngineSpec(PostgresBaseEngineSpec):
engine = 'postgresql' engine = 'postgresql'
@classmethod @classmethod
def get_table_names(cls, schema, inspector): @cache_util.memoized_func(
key=lambda *args, **kwargs: 'db:{db_id}:schema:{schema}:table_list'.format(
db_id=kwargs.get('db_id'), schema=kwargs.get('schema')))
def get_table_names(cls, inspector, db_id, schema,
enable_cache, cache_timeout, force=False):
"""Need to consider foreign tables for PostgreSQL""" """Need to consider foreign tables for PostgreSQL"""
tables = inspector.get_table_names(schema) tables = inspector.get_table_names(schema)
tables.extend(inspector.get_foreign_table_names(schema)) tables.extend(inspector.get_foreign_table_names(schema))
@ -570,7 +583,6 @@ class SqliteEngineSpec(BaseEngineSpec):
@classmethod @classmethod
@cache_util.memoized_func( @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) use_tables_cache=True)
def fetch_result_sets(cls, db, datasource_type, force=False): def fetch_result_sets(cls, db, datasource_type, force=False):
@ -596,7 +608,11 @@ class SqliteEngineSpec(BaseEngineSpec):
return "'{}'".format(iso) return "'{}'".format(iso)
@classmethod @classmethod
def get_table_names(cls, schema, inspector): @cache_util.memoized_func(
key=lambda *args, **kwargs: 'db:{db_id}:schema:{schema}:table_list'.format(
db_id=kwargs.get('db_id'), schema=kwargs.get('schema')))
def get_table_names(cls, inspector, db_id, schema,
enable_cache, cache_timeout, force=False):
"""Need to disregard the schema for Sqlite""" """Need to disregard the schema for Sqlite"""
return sorted(inspector.get_table_names()) return sorted(inspector.get_table_names())
@ -721,7 +737,6 @@ class PrestoEngineSpec(BaseEngineSpec):
@classmethod @classmethod
@cache_util.memoized_func( @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) use_tables_cache=True)
def fetch_result_sets(cls, db, datasource_type, force=False): def fetch_result_sets(cls, db, datasource_type, force=False):
@ -1003,7 +1018,6 @@ class HiveEngineSpec(PrestoEngineSpec):
@classmethod @classmethod
@cache_util.memoized_func( @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) use_tables_cache=True)
def fetch_result_sets(cls, db, datasource_type, force=False): def fetch_result_sets(cls, db, datasource_type, force=False):
@ -1461,8 +1475,6 @@ class ImpalaEngineSpec(BaseEngineSpec):
@classmethod @classmethod
@cache_util.memoized_func( @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'))) key=lambda *args, **kwargs: 'db:{}:schema_list'.format(kwargs.get('db_id')))
def get_schema_names(cls, inspector, db_id, def get_schema_names(cls, inspector, db_id,
enable_cache, cache_timeout, force=False): enable_cache, cache_timeout, force=False):

View File

@ -847,8 +847,18 @@ class Database(Model, AuditMixinNullable, ImportMixin):
tables_dict = self.db_engine_spec.fetch_result_sets( tables_dict = self.db_engine_spec.fetch_result_sets(
self, 'table', force=force) self, 'table', force=force)
return tables_dict.get('', []) return tables_dict.get('', [])
return sorted(
self.db_engine_spec.get_table_names(schema, self.inspector)) extra = self.get_extra()
medatada_cache_timeout = extra.get('metadata_cache_timeout', {})
table_cache_timeout = medatada_cache_timeout.get('table_cache_timeout')
enable_cache = 'table_cache_timeout' in medatada_cache_timeout
return sorted(self.db_engine_spec.get_table_names(
inspector=self.inspector,
db_id=self.id,
schema=schema,
enable_cache=enable_cache,
cache_timeout=table_cache_timeout,
force=force))
def all_view_names(self, schema=None, force=False): def all_view_names(self, schema=None, force=False):
if not schema: if not schema:
@ -859,7 +869,17 @@ class Database(Model, AuditMixinNullable, ImportMixin):
return views_dict.get('', []) return views_dict.get('', [])
views = [] views = []
try: try:
views = self.inspector.get_view_names(schema) extra = self.get_extra()
medatada_cache_timeout = extra.get('metadata_cache_timeout', {})
table_cache_timeout = medatada_cache_timeout.get('table_cache_timeout')
enable_cache = 'table_cache_timeout' in medatada_cache_timeout
views = self.db_engine_spec.get_view_names(
inspector=self.inspector,
db_id=self.id,
schema=schema,
enable_cache=enable_cache,
cache_timeout=table_cache_timeout,
force=force)
except Exception: except Exception:
pass pass
return views return views

View File

@ -206,7 +206,8 @@ class DatabaseView(SupersetModelView, DeleteMixin, YamlExportMixin): # noqa
'#sqlalchemy.schema.MetaData) call.<br/>' '#sqlalchemy.schema.MetaData) call.<br/>'
'2. The ``metadata_cache_timeout`` is a cache timeout setting ' '2. The ``metadata_cache_timeout`` is a cache timeout setting '
'in seconds for metadata fetch of this database. Specify it as ' 'in seconds for metadata fetch of this database. Specify it as '
'**"metadata_cache_timeout": {"schema_cache_timeout": 600}**. ' '**"metadata_cache_timeout": {"schema_cache_timeout": 600, '
'"table_cache_timeout": 600}**. '
'If unset, cache will not be enabled for the functionality. ' 'If unset, cache will not be enabled for the functionality. '
'A timeout of 0 indicates that the cache never expires.<br/>' 'A timeout of 0 indicates that the cache never expires.<br/>'
'3. The ``schemas_allowed_for_csv_upload`` is a comma separated list ' '3. The ``schemas_allowed_for_csv_upload`` is a comma separated list '
@ -1538,7 +1539,7 @@ class Superset(BaseSupersetView):
@has_access_api @has_access_api
@expose('/schemas/<db_id>/') @expose('/schemas/<db_id>/')
@expose('/schemas/<db_id>/<force_refresh>/') @expose('/schemas/<db_id>/<force_refresh>/')
def schemas(self, db_id, force_refresh='true'): def schemas(self, db_id, force_refresh='false'):
db_id = int(db_id) db_id = int(db_id)
force_refresh = force_refresh.lower() == 'true' force_refresh = force_refresh.lower() == 'true'
database = ( database = (
@ -1556,16 +1557,18 @@ class Superset(BaseSupersetView):
@api @api
@has_access_api @has_access_api
@expose('/tables/<db_id>/<schema>/<substr>/') @expose('/tables/<db_id>/<schema>/<substr>/')
def tables(self, db_id, schema, substr): @expose('/tables/<db_id>/<schema>/<substr>/<force_refresh>/')
def tables(self, db_id, schema, substr, force_refresh='false'):
"""Endpoint to fetch the list of tables for given database""" """Endpoint to fetch the list of tables for given database"""
db_id = int(db_id) db_id = int(db_id)
force_refresh = force_refresh.lower() == 'true'
schema = utils.js_string_to_python(schema) schema = utils.js_string_to_python(schema)
substr = utils.js_string_to_python(substr) substr = utils.js_string_to_python(substr)
database = db.session.query(models.Database).filter_by(id=db_id).one() database = db.session.query(models.Database).filter_by(id=db_id).one()
table_names = security_manager.accessible_by_user( table_names = security_manager.accessible_by_user(
database, database.all_table_names(schema), schema) database, database.all_table_names(schema, force_refresh), schema)
view_names = security_manager.accessible_by_user( view_names = security_manager.accessible_by_user(
database, database.all_view_names(schema), schema) database, database.all_view_names(schema, force_refresh), schema)
if substr: if substr:
table_names = [tn for tn in table_names if substr in tn] table_names = [tn for tn in table_names if substr in tn]