Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow user to force refresh metadata #5933

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for fixing this!

npm install
npm run test
```
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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', () => {
Expand Down
50 changes: 32 additions & 18 deletions superset/assets/src/SqlLab/components/SqlEditorLeftBar.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand Down Expand Up @@ -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);
Expand All @@ -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) {
Expand Down Expand Up @@ -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 });
Expand All @@ -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>
Expand Down Expand Up @@ -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">
Expand Down
51 changes: 51 additions & 0 deletions superset/assets/src/components/RefreshLabel.jsx
Original file line number Diff line number Diff line change
@@ -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;
36 changes: 31 additions & 5 deletions superset/cache_util.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
35 changes: 29 additions & 6 deletions superset/db_engine_specs.py
Original file line number Diff line number Diff line change
Expand Up @@ -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]}.

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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 = {}
Expand Down Expand Up @@ -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]}.

Expand Down Expand Up @@ -979,7 +996,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)
Expand Down Expand Up @@ -1434,7 +1452,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
Expand Down
14 changes: 12 additions & 2 deletions superset/models/core.py
Original file line number Diff line number Diff line change
Expand Up @@ -647,6 +647,7 @@ class Database(Model, AuditMixinNullable, ImportMixin):
{
"metadata_params": {},
"engine_params": {},
"metadata_cache_timeout": {},
"schemas_allowed_for_csv_upload": []
}
"""))
Expand Down Expand Up @@ -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):
Expand Down
20 changes: 14 additions & 6 deletions superset/views/core.py
Original file line number Diff line number Diff line change
Expand Up @@ -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 '
Expand All @@ -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': _(
Expand All @@ -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'),
Expand All @@ -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))

Expand Down Expand Up @@ -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}),
Expand Down