From 5962f2e6552e19f4327e32d52bdb16823abcf4c4 Mon Sep 17 00:00:00 2001 From: Bogdan Kyryliuk Date: Tue, 23 Aug 2016 19:59:26 -0700 Subject: [PATCH 1/4] First changes for the sync --- .../SqlLab/components/QueryAutoRefresh.jsx | 12 +-- .../SqlLab/components/QueryHistory.jsx | 5 +- .../SqlLab/components/QueryTable.jsx | 14 +-- .../SqlLab/components/ResultSet.jsx | 2 +- .../SqlLab/components/SqlEditor.jsx | 57 +++++++--- caravel/assets/javascripts/SqlLab/reducers.js | 7 +- .../versions/ad82a75afd82_add_query_model.py | 1 + caravel/models.py | 24 +++-- caravel/sql_lab.py | 10 +- caravel/sql_lab_utils.py | 2 +- caravel/views.py | 102 ++++++++++-------- 11 files changed, 143 insertions(+), 93 deletions(-) diff --git a/caravel/assets/javascripts/SqlLab/components/QueryAutoRefresh.jsx b/caravel/assets/javascripts/SqlLab/components/QueryAutoRefresh.jsx index cccb6bfb8c196..a5b29604df5a4 100644 --- a/caravel/assets/javascripts/SqlLab/components/QueryAutoRefresh.jsx +++ b/caravel/assets/javascripts/SqlLab/components/QueryAutoRefresh.jsx @@ -16,7 +16,7 @@ class QueryAutoRefresh extends React.Component { } startTimer() { if (!(this.timer)) { - this.timer = setInterval(this.stopwatch.bind(this), 2000); + this.timer = setInterval(this.stopwatch.bind(this), 1000); } } stopTimer() { @@ -25,26 +25,24 @@ class QueryAutoRefresh extends React.Component { } stopwatch() { const url = '/caravel/queries/0'; + // No updates in case of failure. $.getJSON( url, (data, status, xhr) => { - if (status == 200) { - console.log("Fetched queries") - console.log(data) + if (status === "success") { this.props.actions.refreshQueries(data); } }); - console.log(new Date()); } render() { return null; } } QueryAutoRefresh.propTypes = { - query: React.PropTypes.object, + queries: React.PropTypes.array, }; QueryAutoRefresh.defaultProps = { - query: null, + queries: null, }; function mapStateToProps() { diff --git a/caravel/assets/javascripts/SqlLab/components/QueryHistory.jsx b/caravel/assets/javascripts/SqlLab/components/QueryHistory.jsx index 3d591790a9785..fd75457125bb3 100644 --- a/caravel/assets/javascripts/SqlLab/components/QueryHistory.jsx +++ b/caravel/assets/javascripts/SqlLab/components/QueryHistory.jsx @@ -18,7 +18,8 @@ const QueryHistory = (props) => { if (queriesArray.length > 0) { return ( ); @@ -31,7 +32,7 @@ const QueryHistory = (props) => { }; QueryHistory.defaultProps = { - queries: [], + queries: {}, }; QueryHistory.propTypes = { diff --git a/caravel/assets/javascripts/SqlLab/components/QueryTable.jsx b/caravel/assets/javascripts/SqlLab/components/QueryTable.jsx index 028352dc572b9..841e50d285f17 100644 --- a/caravel/assets/javascripts/SqlLab/components/QueryTable.jsx +++ b/caravel/assets/javascripts/SqlLab/components/QueryTable.jsx @@ -36,13 +36,13 @@ class QueryTable extends React.Component { render() { const data = this.props.queries.map((query) => { const q = Object.assign({}, query); - const since = (q.endDttm) ? q.endDttm : new Date(); - let duration = since.valueOf() - q.startDttm.valueOf(); - duration = moment.utc(duration); + // TODO(bkyryliuk): rename ...Dttm into the ...Timestamp. + const since = (q.endDttm) ? q.endDttm : new Date().getTime(); + const duration = moment.utc(since - q.startDttm); if (q.endDttm) { q.duration = duration.format('HH:mm:ss.SS'); } - q.started = moment(q.startDttm).format('HH:mm:ss'); + q.started = moment.utc(q.startDttm).format('HH:mm:ss'); q.sql = {q.sql}; q.state = ( @@ -103,10 +103,10 @@ class QueryTable extends React.Component { QueryTable.propTypes = { columns: React.PropTypes.array, actions: React.PropTypes.object, - queries: React.PropTypes.object, + queries: React.PropTypes.array, }; QueryTable.defaultProps = { - columns: ['state', 'started', 'duration', 'rows', 'sql', 'actions'], + columns: ['state', 'started', 'duration', 'progress', 'rows', 'sql', 'actions'], queries: [], }; diff --git a/caravel/assets/javascripts/SqlLab/components/ResultSet.jsx b/caravel/assets/javascripts/SqlLab/components/ResultSet.jsx index 1414d1d4b720a..25ed558e6ce2f 100644 --- a/caravel/assets/javascripts/SqlLab/components/ResultSet.jsx +++ b/caravel/assets/javascripts/SqlLab/components/ResultSet.jsx @@ -53,7 +53,7 @@ class ResultSet extends React.Component { ); } - if (results.data.length > 0) { + if (results && results.data.length > 0) { return (
self._query.progress: self._query.progress = progress - self._session.flush() + self._session.commit() query_stats = cursor.poll() # TODO(b.kyryliuk): check for the kill signal. diff --git a/caravel/sql_lab_utils.py b/caravel/sql_lab_utils.py index 085a953667ac0..9e1c21b460fbe 100644 --- a/caravel/sql_lab_utils.py +++ b/caravel/sql_lab_utils.py @@ -14,7 +14,7 @@ def create_scoped_session(): engine = create_engine( app.config.get('SQLALCHEMY_DATABASE_URI'), convert_unicode=True) return scoped_session(sessionmaker( - autocommit=True, autoflush=False, bind=engine)) + autocommit=False, autoflush=False, bind=engine)) def fetch_response_from_cursor(result_proxy): diff --git a/caravel/views.py b/caravel/views.py index d93726b9536de..9ed7247b65c34 100755 --- a/caravel/views.py +++ b/caravel/views.py @@ -1273,7 +1273,7 @@ def sqllab_viz(self): table = models.SqlaTable( table_name=table_name, ) - table.database_id = data.get('dbId') + table.database_id = data.get('databaseId') table.sql = data.get('sql') db.session.add(table) cols = [] @@ -1444,45 +1444,23 @@ def theme(self): return self.render_template('caravel/theme.html') @has_access - @expose("/sql_json/", methods=['POST', 'GET']) + @expose("/create_query/", methods=['POST', 'GET']) @log_this - def sql_json(self): - """Runs arbitrary sql and returns and json""" + def create_query(self): sql = request.form.get('sql') database_id = request.form.get('database_id') schema = request.form.get('schema') - tab_name = request.form.get('tab_name') + tab_name = request.form.get('tab') + sql_editor_id = request.form.get('sql_editor_id') - async = request.form.get('async') == 'true' tmp_table_name = request.form.get('tmp_table_name', None) select_as_cta = request.form.get('select_as_cta') == 'true' - print(request.form.get('async')) - print(type(request.form.get('async'))) - - session = db.session() - mydb = session.query(models.Database).filter_by(id=database_id).first() - - if not mydb: - return Response( - json.dumps({ - 'error': 'Database with id {} is missing.'.format( - database_id), - 'status': models.QueryStatus.FAILED, - }), - status=500, - mimetype="application/json") - - if not (self.can_access( - 'all_datasource_access', 'all_datasource_access') or - self.can_access('database_access', mydb.perm)): - raise utils.CaravelSecurityException(_( - "SQL Lab requires the `all_datasource_access` or " - "specific DB permission")) start_time = datetime.now() query_name = '{}_{}_{}'.format( g.user.get_id(), tab_name, start_time.strftime('%M:%S:%f')) + s = db.session() query = models.Query( database_id=database_id, limit=app.config.get('SQL_MAX_ROW', None), @@ -1492,38 +1470,77 @@ def sql_json(self): # TODO(bkyryliuk): consider it being DB property. select_as_cta=select_as_cta, start_time=start_time, - status=models.QueryStatus.SCHEDULED, + status=models.QueryStatus.IN_PROGRESS, tab_name=tab_name, + sql_editor_id=sql_editor_id, tmp_table_name=tmp_table_name, user_id=g.user.get_id(), ) - session.add(query) - session.commit() + s.add(query) + s.commit() + return Response( + json.dumps({'query': query.to_dict()}, + default=utils.json_int_dttm_ser, + allow_nan=False), + status=200, + mimetype="application/json") + + + + @has_access + @expose("/sql_json/", methods=['POST', 'GET']) + @log_this + def sql_json(self): + """Runs arbitrary sql and returns and json""" + query_id = json.loads(request.form.get('query_id')) + async = request.form.get('async') == 'true' + + s = db.session() + query = s.query(models.Query).filter_by(id=int(query_id)).first() + mydb = s.query(models.Database).filter_by(id=query.database_id).first() + + if not mydb: + return Response( + json.dumps({ + 'error': 'Database with id {} is missing.'.format( + database_id), + 'status': models.QueryStatus.FAILED, + }), + status=500, + mimetype="application/json") + + if not (self.can_access( + 'all_datasource_access', 'all_datasource_access') or + self.can_access('database_access', mydb.perm)): + raise utils.CaravelSecurityException(_( + "SQL Lab requires the `all_datasource_access` or " + "specific DB permission")) # Async request. if async: # Ignore the celery future object and the request may time out. sql_lab.get_sql_results.delay(query.id) - return Response(json.dumps( - { - 'query_id': query.id, - 'status': query.status, - }, - default=utils.json_int_dttm_ser, allow_nan=False), + return Response( + json.dumps({'query': query.to_dict()}, + default=utils.json_int_dttm_ser, + allow_nan=False), status=202, # Accepted mimetype="application/json") # Sync request. data = sql_lab.get_sql_results(query.id) + s.close() + s = db.session() + query = s.query(models.Query).filter_by(id=query.id).first() + data['query'] = query.to_dict() if data['status'] == models.QueryStatus.FAILED: return Response( - json.dumps( - data, default=utils.json_int_dttm_ser, allow_nan=False), + json.dumps(data, default=utils.json_int_dttm_ser, + allow_nan=False), status=500, mimetype="application/json") return Response( - json.dumps( - data, default=utils.json_int_dttm_ser, allow_nan=False), + json.dumps(data, default=utils.json_int_dttm_ser, allow_nan=False), status=200, mimetype="application/json") @@ -1569,7 +1586,6 @@ def csv(self, query_id): @log_this def queries(self, last_updated_ms): """Get the updated queries.""" - if not g.user.get_id(): return Response( json.dumps({'error': "Please login to access the queries."}), @@ -1593,7 +1609,7 @@ def queries(self, last_updated_ms): models.Query.changed_on >= last_updated_dt) if sql_queries: - dict_queries = [q.to_dict() for q in sql_queries] + dict_queries = {q.id: q.to_dict() for q in sql_queries} return Response( json.dumps(dict_queries, default=utils.json_int_dttm_ser), status=200, From 003c9ab2cff7539adf33aefaee08cb9ccd77d8ce Mon Sep 17 00:00:00 2001 From: Bogdan Kyryliuk Date: Wed, 24 Aug 2016 16:10:39 -0700 Subject: [PATCH 2/4] Address comments --- .../javascripts/SqlLab/components/QueryHistory.jsx | 3 +-- .../javascripts/SqlLab/components/SqlEditor.jsx | 11 +---------- caravel/assets/javascripts/SqlLab/reducers.js | 3 +-- caravel/sql_lab_utils.py | 2 +- 4 files changed, 4 insertions(+), 15 deletions(-) diff --git a/caravel/assets/javascripts/SqlLab/components/QueryHistory.jsx b/caravel/assets/javascripts/SqlLab/components/QueryHistory.jsx index fd75457125bb3..af6cb0da0863e 100644 --- a/caravel/assets/javascripts/SqlLab/components/QueryHistory.jsx +++ b/caravel/assets/javascripts/SqlLab/components/QueryHistory.jsx @@ -18,8 +18,7 @@ const QueryHistory = (props) => { if (queriesArray.length > 0) { return ( ); diff --git a/caravel/assets/javascripts/SqlLab/components/SqlEditor.jsx b/caravel/assets/javascripts/SqlLab/components/SqlEditor.jsx index 5e2f788a228ee..da863c205d15c 100644 --- a/caravel/assets/javascripts/SqlLab/components/SqlEditor.jsx +++ b/caravel/assets/javascripts/SqlLab/components/SqlEditor.jsx @@ -56,12 +56,6 @@ class SqlEditor extends React.Component { } startQuery(runAsync = false, ctas = false) { const that = this; - let query = { - sqlEditorId: this.props.queryEditor.id, - sql: this.props.queryEditor.sql, - tab: this.props.queryEditor.title, - dbId: this.props.queryEditor.dbId, - }; const createQueryRequest = { sql: this.props.queryEditor.sql, database_id: this.props.queryEditor.dbId, @@ -78,8 +72,7 @@ class SqlEditor extends React.Component { url: createQueryUrl, data: createQueryRequest, success(results) { - query = Object.assign({}, query, results['query']) - + const query = Object.assign({}, results['query']) // Execute the Query that.props.actions.startQuery(query); @@ -98,7 +91,6 @@ class SqlEditor extends React.Component { if (runAsync) { // TODO nothing? } else { - query = Object.assign({}, query, results['query']) that.props.actions.querySuccess(query, results); } }, @@ -109,7 +101,6 @@ class SqlEditor extends React.Component { } catch (e) { msg = (err.responseText) ? err.responseText : e; } - query = Object.assign({}, query, results['query']) that.props.actions.queryFailed(query, msg); }, }); diff --git a/caravel/assets/javascripts/SqlLab/reducers.js b/caravel/assets/javascripts/SqlLab/reducers.js index 497391cc43314..d3c925e3af4bc 100644 --- a/caravel/assets/javascripts/SqlLab/reducers.js +++ b/caravel/assets/javascripts/SqlLab/reducers.js @@ -192,8 +192,7 @@ export const sqlLabReducer = function (state, action) { [actions.REFRESH_QUERIES]() { const newQueries = Object.assign({}, state['queries'], action.alteredQueries); - console.log(newQueries) - return Object.assign({}, state, { ['queries']: newQueries }); + return Object.assign({}, state, { queries: newQueries }); }, }; if (action.type in actionHandlers) { diff --git a/caravel/sql_lab_utils.py b/caravel/sql_lab_utils.py index 9e1c21b460fbe..085a953667ac0 100644 --- a/caravel/sql_lab_utils.py +++ b/caravel/sql_lab_utils.py @@ -14,7 +14,7 @@ def create_scoped_session(): engine = create_engine( app.config.get('SQLALCHEMY_DATABASE_URI'), convert_unicode=True) return scoped_session(sessionmaker( - autocommit=False, autoflush=False, bind=engine)) + autocommit=True, autoflush=False, bind=engine)) def fetch_response_from_cursor(result_proxy): From 76427ae66e457e744d740eb2311af6608506f320 Mon Sep 17 00:00:00 2001 From: Bogdan Kyryliuk Date: Wed, 24 Aug 2016 16:59:20 -0700 Subject: [PATCH 3/4] Fix query deletions. Update only the queries from the store. --- caravel/assets/javascripts/SqlLab/reducers.js | 22 ++++++++----------- caravel/sql_lab.py | 2 -- caravel/sql_lab_utils.py | 2 +- 3 files changed, 10 insertions(+), 16 deletions(-) diff --git a/caravel/assets/javascripts/SqlLab/reducers.js b/caravel/assets/javascripts/SqlLab/reducers.js index d3c925e3af4bc..3574482b19b33 100644 --- a/caravel/assets/javascripts/SqlLab/reducers.js +++ b/caravel/assets/javascripts/SqlLab/reducers.js @@ -21,16 +21,6 @@ export const initialState = { workspaceQueries: [], }; -function removeFromObject(state, arrKey, obj, idKey = 'id') { - const newObject = {}; - for (const key in state[arrKey]) { - if (!(key === obj[idKey])) { - newObject[idKey] = state[arrKey][idKey]; - } - } - return Object.assign({}, state, { [arrKey]: newObject }); -} - function addToObject(state, arrKey, obj) { const newObject = Object.assign({}, state[arrKey]); const copiedObject = Object.assign({}, obj); @@ -116,7 +106,9 @@ export const sqlLabReducer = function (state, action) { return newState; }, [actions.REMOVE_QUERY]() { - return removeFromObject(state, 'queries', action.query); + const newQueries = Object.assign({}, state['queries']); + delete newQueries[action.query.id] + return Object.assign({}, state, { queries: newQueries }); }, [actions.RESET_STATE]() { return Object.assign({}, initialState); @@ -190,8 +182,12 @@ export const sqlLabReducer = function (state, action) { return removeFromArr(state, 'alerts', action.alert); }, [actions.REFRESH_QUERIES]() { - const newQueries = Object.assign({}, state['queries'], - action.alteredQueries); + const newQueries = Object.assign({}, state['queries']); + // Fetch the updates to the queries present in the store. + for (var query_id in state['queries']) { + newQueries[query_id] = Object.assign(newQueries[query_id], + action.alteredQueries[query_id]) + } return Object.assign({}, state, { queries: newQueries }); }, }; diff --git a/caravel/sql_lab.py b/caravel/sql_lab.py index 31b17b4de8caf..fce615b78397a 100644 --- a/caravel/sql_lab.py +++ b/caravel/sql_lab.py @@ -115,8 +115,6 @@ def _get_sql_results(self, engine): cursor = result_proxy.cursor if hasattr(cursor, "poll"): query_stats = cursor.poll() - self._query.status = models.QueryStatus.IN_PROGRESS - self._session.commit() # poll returns dict -- JSON status information or ``None`` # if the query is done # https://github.com/dropbox/PyHive/blob/ diff --git a/caravel/sql_lab_utils.py b/caravel/sql_lab_utils.py index 085a953667ac0..9e1c21b460fbe 100644 --- a/caravel/sql_lab_utils.py +++ b/caravel/sql_lab_utils.py @@ -14,7 +14,7 @@ def create_scoped_session(): engine = create_engine( app.config.get('SQLALCHEMY_DATABASE_URI'), convert_unicode=True) return scoped_session(sessionmaker( - autocommit=True, autoflush=False, bind=engine)) + autocommit=False, autoflush=False, bind=engine)) def fetch_response_from_cursor(result_proxy): From 258639766503da1638978f6bbdf5e2fa68f5c5ca Mon Sep 17 00:00:00 2001 From: Bogdan Kyryliuk Date: Wed, 24 Aug 2016 17:52:02 -0700 Subject: [PATCH 4/4] Sync queries using tmp_id. --- .../SqlLab/components/QueryAutoRefresh.jsx | 4 +- .../SqlLab/components/SqlEditor.jsx | 64 ++++++++----------- caravel/assets/javascripts/SqlLab/reducers.js | 7 +- caravel/config.py | 4 +- .../versions/ad82a75afd82_add_query_model.py | 1 + caravel/models.py | 4 +- caravel/views.py | 53 +++++++-------- 7 files changed, 67 insertions(+), 70 deletions(-) diff --git a/caravel/assets/javascripts/SqlLab/components/QueryAutoRefresh.jsx b/caravel/assets/javascripts/SqlLab/components/QueryAutoRefresh.jsx index a5b29604df5a4..036579670c97a 100644 --- a/caravel/assets/javascripts/SqlLab/components/QueryAutoRefresh.jsx +++ b/caravel/assets/javascripts/SqlLab/components/QueryAutoRefresh.jsx @@ -39,10 +39,10 @@ class QueryAutoRefresh extends React.Component { } } QueryAutoRefresh.propTypes = { - queries: React.PropTypes.array, + // queries: React.PropTypes.object, }; QueryAutoRefresh.defaultProps = { - queries: null, + // queries: null, }; function mapStateToProps() { diff --git a/caravel/assets/javascripts/SqlLab/components/SqlEditor.jsx b/caravel/assets/javascripts/SqlLab/components/SqlEditor.jsx index da863c205d15c..6a0b8cff4fd03 100644 --- a/caravel/assets/javascripts/SqlLab/components/SqlEditor.jsx +++ b/caravel/assets/javascripts/SqlLab/components/SqlEditor.jsx @@ -56,7 +56,25 @@ class SqlEditor extends React.Component { } startQuery(runAsync = false, ctas = false) { const that = this; - const createQueryRequest = { + + const query = { + id: shortid.generate(), + sqlEditorId: this.props.queryEditor.id, + sql: this.props.queryEditor.sql, + state: 'running', + tab: this.props.queryEditor.title, + dbId: this.props.queryEditor.dbId, + startDttm: new Date(), + }; + + // Execute the Query + that.props.actions.startQuery(query); + + const sqlJsonUrl = '/caravel/sql_json/'; + const sqlJsonRequest = { + json: true, + client_id: query.id, + async: runAsync, sql: this.props.queryEditor.sql, database_id: this.props.queryEditor.dbId, sql_editor_id: this.props.queryEditor.id, @@ -65,45 +83,17 @@ class SqlEditor extends React.Component { json: true, select_as_cta: ctas, }; - const createQueryUrl = '/caravel/create_query/'; $.ajax({ type: 'POST', dataType: 'json', - url: createQueryUrl, - data: createQueryRequest, + url: sqlJsonUrl, + data: sqlJsonRequest, success(results) { - const query = Object.assign({}, results['query']) - // Execute the Query - that.props.actions.startQuery(query); - - const sqlJsonUrl = '/caravel/sql_json/'; - const sqlJsonRequest = { - query_id: query.id, - json: true, - async: runAsync, - }; - $.ajax({ - type: 'POST', - dataType: 'json', - url: sqlJsonUrl, - data: sqlJsonRequest, - success(results) { - if (runAsync) { - // TODO nothing? - } else { - that.props.actions.querySuccess(query, results); - } - }, - error(err) { - let msg = ''; - try { - msg = err.responseJSON.error; - } catch (e) { - msg = (err.responseText) ? err.responseText : e; - } - that.props.actions.queryFailed(query, msg); - }, - }); + if (runAsync) { + // TODO nothing? + } else { + that.props.actions.querySuccess(query, results); + } }, error(err) { let msg = ''; @@ -148,6 +138,8 @@ class SqlEditor extends React.Component { ); + console.log("this.props.latestQuery") + console.log(this.props.latestQuery) if (this.props.latestQuery && this.props.latestQuery.state === 'running') { runButtons = ( diff --git a/caravel/assets/javascripts/SqlLab/reducers.js b/caravel/assets/javascripts/SqlLab/reducers.js index 3574482b19b33..96615e9149233 100644 --- a/caravel/assets/javascripts/SqlLab/reducers.js +++ b/caravel/assets/javascripts/SqlLab/reducers.js @@ -138,6 +138,7 @@ export const sqlLabReducer = function (state, action) { state: 'success', results: action.results, rows: action.results.data.length, + endDttm: moment(), }; return alterInObject(state, 'queries', action.query, alts); }, @@ -184,9 +185,9 @@ export const sqlLabReducer = function (state, action) { [actions.REFRESH_QUERIES]() { const newQueries = Object.assign({}, state['queries']); // Fetch the updates to the queries present in the store. - for (var query_id in state['queries']) { - newQueries[query_id] = Object.assign(newQueries[query_id], - action.alteredQueries[query_id]) + for (var queryId in state['queries']) { + newQueries[queryId] = Object.assign(newQueries[queryId], + action.alteredQueries[queryId]) } return Object.assign({}, state, { queries: newQueries }); }, diff --git a/caravel/config.py b/caravel/config.py index 6cb76aa09c4db..fd2062e68eca4 100644 --- a/caravel/config.py +++ b/caravel/config.py @@ -36,8 +36,8 @@ SECRET_KEY = '\2\1thisismyscretkey\1\2\e\y\y\h' # noqa # The SQLAlchemy connection string. -# SQLALCHEMY_DATABASE_URI = 'sqlite:///' + os.path.join(DATA_DIR, 'caravel.db') -SQLALCHEMY_DATABASE_URI = 'mysql://root@localhost/caravel_db' +SQLALCHEMY_DATABASE_URI = 'sqlite:///' + os.path.join(DATA_DIR, 'caravel.db') +# SQLALCHEMY_DATABASE_URI = 'mysql://myapp@localhost/myapp' # SQLALCHEMY_DATABASE_URI = 'postgresql://root:password@localhost/myapp' # Flask-WTF flag for CSRF diff --git a/caravel/migrations/versions/ad82a75afd82_add_query_model.py b/caravel/migrations/versions/ad82a75afd82_add_query_model.py index aaf61bfb35f40..a166bba637465 100644 --- a/caravel/migrations/versions/ad82a75afd82_add_query_model.py +++ b/caravel/migrations/versions/ad82a75afd82_add_query_model.py @@ -17,6 +17,7 @@ def upgrade(): op.create_table('query', sa.Column('id', sa.Integer(), nullable=False), + sa.Column('client_id', sa.String(length=11), nullable=False), sa.Column('database_id', sa.Integer(), nullable=False), sa.Column('tmp_table_name', sa.String(length=256), nullable=True), sa.Column('tab_name', sa.String(length=256),nullable=True), diff --git a/caravel/models.py b/caravel/models.py index 50f4a4a8ee210..c482b90e42ad0 100644 --- a/caravel/models.py +++ b/caravel/models.py @@ -1762,6 +1762,7 @@ class Query(Model): __tablename__ = 'query' id = Column(Integer, primary_key=True) + client_id = Column(String(11)) database_id = Column(Integer, ForeignKey('dbs.id'), nullable=False) @@ -1804,7 +1805,8 @@ class Query(Model): def to_dict(self): return { - 'id': self.id, + 'serverId': self.id, + 'id': self.client_id, 'dbId': self.database_id, 'tab': self.tab_name, 'sqlEditorId': self.sql_editor_id, diff --git a/caravel/views.py b/caravel/views.py index 9ed7247b65c34..3bc5577b8de91 100755 --- a/caravel/views.py +++ b/caravel/views.py @@ -1444,15 +1444,17 @@ def theme(self): return self.render_template('caravel/theme.html') @has_access - @expose("/create_query/", methods=['POST', 'GET']) + @expose("/sql_json/", methods=['POST', 'GET']) @log_this - def create_query(self): + def sql_json(self): + """Runs arbitrary sql and returns and json""" + async = request.form.get('async') == 'true' + client_id = request.form.get('client_id') sql = request.form.get('sql') database_id = request.form.get('database_id') schema = request.form.get('schema') tab_name = request.form.get('tab') sql_editor_id = request.form.get('sql_editor_id') - tmp_table_name = request.form.get('tmp_table_name', None) select_as_cta = request.form.get('select_as_cta') == 'true' @@ -1461,6 +1463,21 @@ def create_query(self): g.user.get_id(), tab_name, start_time.strftime('%M:%S:%f')) s = db.session() + session = db.session() + mydb = session.query(models.Database).filter_by(id=database_id).first() + if not mydb: + return Response(json.dumps({ + 'error': 'Database with id {} is missing.'.format(database_id), + 'status': models.QueryStatus.FAILED, + }), + status=500, + mimetype="application/json" + ) + if not (self.can_access('all_datasource_access', 'all_datasource_access') or + self.can_access('database_access', mydb.perm)): + raise utils.CaravelSecurityException(_( + "SQL Lab requires the `all_datasource_access` or specific DB permission")) + query = models.Query( database_id=database_id, limit=app.config.get('SQL_MAX_ROW', None), @@ -1475,28 +1492,13 @@ def create_query(self): sql_editor_id=sql_editor_id, tmp_table_name=tmp_table_name, user_id=g.user.get_id(), + client_id=client_id, ) s.add(query) s.commit() - return Response( - json.dumps({'query': query.to_dict()}, - default=utils.json_int_dttm_ser, - allow_nan=False), - status=200, - mimetype="application/json") - - - - @has_access - @expose("/sql_json/", methods=['POST', 'GET']) - @log_this - def sql_json(self): - """Runs arbitrary sql and returns and json""" - query_id = json.loads(request.form.get('query_id')) - async = request.form.get('async') == 'true' s = db.session() - query = s.query(models.Query).filter_by(id=int(query_id)).first() + query = s.query(models.Query).filter_by(id=int(query.id)).first() mydb = s.query(models.Database).filter_by(id=query.database_id).first() if not mydb: @@ -1513,8 +1515,7 @@ def sql_json(self): 'all_datasource_access', 'all_datasource_access') or self.can_access('database_access', mydb.perm)): raise utils.CaravelSecurityException(_( - "SQL Lab requires the `all_datasource_access` or " - "specific DB permission")) + "SQL Lab requires the `all_datasource_access` or specific DB permission")) # Async request. if async: @@ -1535,10 +1536,10 @@ def sql_json(self): data['query'] = query.to_dict() if data['status'] == models.QueryStatus.FAILED: return Response( - json.dumps(data, default=utils.json_int_dttm_ser, - allow_nan=False), + json.dumps(data, default=utils.json_int_dttm_ser, allow_nan=False), status=500, - mimetype="application/json") + mimetype="application/json" + ) return Response( json.dumps(data, default=utils.json_int_dttm_ser, allow_nan=False), status=200, @@ -1609,7 +1610,7 @@ def queries(self, last_updated_ms): models.Query.changed_on >= last_updated_dt) if sql_queries: - dict_queries = {q.id: q.to_dict() for q in sql_queries} + dict_queries = {q.client_id: q.to_dict() for q in sql_queries} return Response( json.dumps(dict_queries, default=utils.json_int_dttm_ser), status=200,