You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by GitBox <gi...@apache.org> on 2018/10/16 20:14:48 UTC
[GitHub] betodealmeida closed pull request #6078: allow cache and force
refresh on table list
betodealmeida closed pull request #6078: allow cache and force refresh on table list
URL: https://github.com/apache/incubator-superset/pull/6078
This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:
As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):
diff --git a/superset/assets/spec/javascripts/sqllab/SqlEditorLeftBar_spec.jsx b/superset/assets/spec/javascripts/sqllab/SqlEditorLeftBar_spec.jsx
index b5f0187384..08fa24af8f 100644
--- a/superset/assets/spec/javascripts/sqllab/SqlEditorLeftBar_spec.jsx
+++ b/superset/assets/spec/javascripts/sqllab/SqlEditorLeftBar_spec.jsx
@@ -41,14 +41,10 @@ describe('SqlEditorLeftBar', () => {
expect(wrapper.find(TableElement)).toHaveLength(1);
});
describe('onDatabaseChange', () => {
- it('should fetch tables', () => {
- sinon.stub(wrapper.instance(), 'fetchTables');
+ it('should fetch schemas', () => {
sinon.stub(wrapper.instance(), 'fetchSchemas');
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);
- wrapper.instance().fetchTables.restore();
wrapper.instance().fetchSchemas.restore();
});
it('should clear tableOptions', () => {
@@ -103,9 +99,9 @@ describe('SqlEditorLeftBar', () => {
d.resolve(tables);
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);
});
it('should handle error', () => {
diff --git a/superset/assets/src/SqlLab/components/SqlEditorLeftBar.jsx b/superset/assets/src/SqlLab/components/SqlEditorLeftBar.jsx
index d8db05cb35..e6e7f5dd26 100644
--- a/superset/assets/src/SqlLab/components/SqlEditorLeftBar.jsx
+++ b/superset/assets/src/SqlLab/components/SqlEditorLeftBar.jsx
@@ -40,13 +40,10 @@ class SqlEditorLeftBar extends React.PureComponent {
}
onDatabaseChange(db, force) {
const val = db ? db.value : null;
- this.setState({ schemaOptions: [] });
+ this.setState({ schemaOptions: [], tableOptions: [] });
this.props.actions.queryEditorSetSchema(this.props.queryEditor, null);
this.props.actions.queryEditorSetDb(this.props.queryEditor, val);
- if (!(db)) {
- this.setState({ tableOptions: [] });
- } else {
- this.fetchTables(val, this.props.queryEditor.schema);
+ if (db) {
this.fetchSchemas(val, force || false);
}
}
@@ -69,11 +66,12 @@ class SqlEditorLeftBar extends React.PureComponent {
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
+ const forceRefresh = force || false;
if (dbId && schema) {
this.setState({ tableLoading: true, tableOptions: [] });
- const url = `/superset/tables/${dbId}/${schema}/${substr}/`;
+ const url = `/superset/tables/${dbId}/${schema}/${substr}/${forceRefresh}/`;
$.get(url).done((data) => {
const filterOptions = createFilterOptions({ options: data.options });
this.setState({
@@ -110,10 +108,10 @@ class SqlEditorLeftBar extends React.PureComponent {
}
this.props.actions.addTable(this.props.queryEditor, tableName, schemaName);
}
- changeSchema(schemaOpt) {
+ changeSchema(schemaOpt, force) {
const schema = (schemaOpt) ? schemaOpt.value : null;
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) {
const actualDbId = dbId || this.props.queryEditor.dbId;
@@ -196,7 +194,7 @@ class SqlEditorLeftBar extends React.PureComponent {
<RefreshLabel
onClick={this.onDatabaseChange.bind(
this, { value: database.id }, true)}
- tooltipContent="force refresh schema list"
+ tooltipContent={t('force refresh schema list')}
/>
</div>
</div>
@@ -214,30 +212,41 @@ class SqlEditorLeftBar extends React.PureComponent {
</i>)
</small>
</ControlLabel>
- {this.props.queryEditor.schema &&
- <Select
- name="select-table"
- ref="selectTable"
- isLoading={this.state.tableLoading}
- placeholder={t('Select table or type table name')}
- autosize={false}
- onChange={this.changeTable.bind(this)}
- filterOptions={this.state.filterOptions}
- options={this.state.tableOptions}
- />
- }
- {!this.props.queryEditor.schema &&
- <Select
- async
- name="async-select-table"
- ref="selectTable"
- placeholder={tableSelectPlaceholder}
- disabled={tableSelectDisabled}
- autosize={false}
- onChange={this.changeTable.bind(this)}
- loadOptions={this.getTableNamesBySubStr.bind(this)}
- />
- }
+ <div className="row">
+ <div className="col-md-11 col-xs-11" style={{ paddingRight: '2px' }}>
+ {this.props.queryEditor.schema &&
+ <Select
+ name="select-table"
+ ref="selectTable"
+ isLoading={this.state.tableLoading}
+ placeholder={t('Select table or type table name')}
+ autosize={false}
+ onChange={this.changeTable.bind(this)}
+ filterOptions={this.state.filterOptions}
+ options={this.state.tableOptions}
+ />
+ }
+ {!this.props.queryEditor.schema &&
+ <Select
+ async
+ name="async-select-table"
+ ref="selectTable"
+ placeholder={tableSelectPlaceholder}
+ disabled={tableSelectDisabled}
+ 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>
<hr />
<div className="m-t-5">
diff --git a/superset/cache_util.py b/superset/cache_util.py
index 12bed2b180..843ffdb0e8 100644
--- a/superset/cache_util.py
+++ b/superset/cache_util.py
@@ -9,25 +9,17 @@ def view_cache_key(*unused_args, **unused_kwargs):
return 'view/{}/{}'.format(request.path, args_hash)
-def default_timeout(*unused_args, **unused_kwargs):
- return 5 * 60
-
-
-def default_enable_cache(*unused_args, **unused_kwargs):
- return True
+def memoized_func(key=view_cache_key, use_tables_cache=False):
+ """Use this decorator to cache functions that have predefined first arg.
+ enable_cache is treated as True by default,
+ except enable_cache = False is passed to the decorated function.
-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.
+ force means whether to force refresh the cache and is treated as False by default,
+ except force = True is passed to the decorated function.
- 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.
+ 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.
Key is a callable function that takes function arguments and
@@ -42,15 +34,16 @@ def wrap(f):
if selected_cache:
def wrapped_f(cls, *args, **kwargs):
- if not enable_cache(*args, **kwargs):
+ if not kwargs.get('enable_cache', True):
return f(cls, *args, **kwargs)
cache_key = key(*args, **kwargs)
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
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
else:
# noop
diff --git a/superset/db_engine_specs.py b/superset/db_engine_specs.py
index 172cf9cc86..20d57d6976 100644
--- a/superset/db_engine_specs.py
+++ b/superset/db_engine_specs.py
@@ -228,7 +228,6 @@ 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]),
use_tables_cache=True)
def fetch_result_sets(cls, db, datasource_type, force=False):
@@ -295,8 +294,6 @@ def patch(cls):
@classmethod
@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):
@@ -312,9 +309,21 @@ def get_schema_names(cls, inspector, db_id,
return inspector.get_schema_names()
@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))
+ @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
def where_latest_partition(
cls, table_name, schema, database, qry, columns=None):
@@ -438,7 +447,11 @@ class PostgresEngineSpec(PostgresBaseEngineSpec):
engine = 'postgresql'
@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"""
tables = inspector.get_table_names(schema)
tables.extend(inspector.get_foreign_table_names(schema))
@@ -570,7 +583,6 @@ def epoch_to_dttm(cls):
@classmethod
@cache_util.memoized_func(
- timeout=600,
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):
@@ -596,7 +608,11 @@ def convert_dttm(cls, target_type, dttm):
return "'{}'".format(iso)
@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"""
return sorted(inspector.get_table_names())
@@ -721,7 +737,6 @@ def epoch_to_dttm(cls):
@classmethod
@cache_util.memoized_func(
- timeout=600,
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):
@@ -1003,7 +1018,6 @@ def patch(cls):
@classmethod
@cache_util.memoized_func(
- timeout=600,
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):
@@ -1461,8 +1475,6 @@ def convert_dttm(cls, target_type, dttm):
@classmethod
@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):
diff --git a/superset/models/core.py b/superset/models/core.py
index 7e1aaf4d4e..2bc0206ef5 100644
--- a/superset/models/core.py
+++ b/superset/models/core.py
@@ -847,8 +847,18 @@ def all_table_names(self, schema=None, force=False):
tables_dict = self.db_engine_spec.fetch_result_sets(
self, 'table', force=force)
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):
if not schema:
@@ -859,7 +869,17 @@ def all_view_names(self, schema=None, force=False):
return views_dict.get('', [])
views = []
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:
pass
return views
diff --git a/superset/views/core.py b/superset/views/core.py
index 05ef4315e2..00288b21bc 100755
--- a/superset/views/core.py
+++ b/superset/views/core.py
@@ -206,7 +206,8 @@ class DatabaseView(SupersetModelView, DeleteMixin, YamlExportMixin): # noqa
'#sqlalchemy.schema.MetaData) call.<br/>'
'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}**. '
+ '**"metadata_cache_timeout": {"schema_cache_timeout": 600, '
+ '"table_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 '
@@ -1538,7 +1539,7 @@ def checkbox(self, model_view, id_, attr, value):
@has_access_api
@expose('/schemas/<db_id>/')
@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)
force_refresh = force_refresh.lower() == 'true'
database = (
@@ -1556,16 +1557,18 @@ def schemas(self, db_id, force_refresh='true'):
@api
@has_access_api
@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"""
db_id = int(db_id)
+ force_refresh = force_refresh.lower() == 'true'
schema = utils.js_string_to_python(schema)
substr = utils.js_string_to_python(substr)
database = db.session.query(models.Database).filter_by(id=db_id).one()
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(
- database, database.all_view_names(schema), schema)
+ database, database.all_view_names(schema, force_refresh), schema)
if substr:
table_names = [tn for tn in table_names if substr in tn]
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org