-
Notifications
You must be signed in to change notification settings - Fork 16.6k
More improvements to SQL Lab #1104
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
Changes from all commits
26dcf89
5b88c1d
733a63d
66ad2da
a4a0657
be834f7
1c73615
5c33a55
1fbbed9
d67477c
4ae6f0a
7ee8d6b
de95775
8b3b5f7
ca351cf
a64ee9b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -9,9 +9,7 @@ import { | |
| InputGroup, | ||
| Form, | ||
| FormControl, | ||
| DropdownButton, | ||
| Label, | ||
| MenuItem, | ||
| OverlayTrigger, | ||
| Row, | ||
| Tooltip, | ||
|
|
@@ -27,7 +25,6 @@ import { connect } from 'react-redux'; | |
| import * as Actions from '../actions'; | ||
|
|
||
| import shortid from 'shortid'; | ||
| import ButtonWithTooltip from './ButtonWithTooltip'; | ||
| import SouthPane from './SouthPane'; | ||
| import Timer from './Timer'; | ||
|
|
||
|
|
@@ -52,8 +49,8 @@ class SqlEditor extends React.Component { | |
| this.startQuery(); | ||
| } | ||
| } | ||
| runQuery() { | ||
| this.startQuery(); | ||
| runQuery(runAsync = false) { | ||
| this.startQuery(runAsync); | ||
| } | ||
| startQuery(runAsync = false, ctas = false) { | ||
| const that = this; | ||
|
|
@@ -76,10 +73,10 @@ class SqlEditor extends React.Component { | |
|
|
||
| const sqlJsonUrl = '/caravel/sql_json/'; | ||
| const sqlJsonRequest = { | ||
| async: runAsync, | ||
| client_id: query.id, | ||
| database_id: this.props.queryEditor.dbId, | ||
| json: true, | ||
| runAsync, | ||
| schema: this.props.queryEditor.schema, | ||
| select_as_cta: ctas, | ||
| sql: this.props.queryEditor.sql, | ||
|
|
@@ -149,17 +146,36 @@ class SqlEditor extends React.Component { | |
| } | ||
|
|
||
| render() { | ||
| let runButtons = ( | ||
| <ButtonGroup bsSize="small" className="inline m-r-5 pull-left"> | ||
| let runButtons = []; | ||
| if (this.props.database && this.props.database.allow_run_sync) { | ||
| runButtons.push( | ||
| <Button | ||
| bsSize="small" | ||
| bsStyle="primary" | ||
| style={{ width: '100px' }} | ||
| onClick={this.runQuery.bind(this)} | ||
| onClick={this.runQuery.bind(this, false)} | ||
| disabled={!(this.props.queryEditor.dbId)} | ||
| > | ||
| <i className="fa fa-table" /> Run Query | ||
| </Button> | ||
| ); | ||
| } | ||
| if (this.props.database && this.props.database.allow_run_async) { | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. should this be an |
||
| runButtons.push( | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. would be nice to DRY this up. could make a There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. could probably simply further by only passing the async prop. if async is false, use sync. |
||
| <Button | ||
| bsSize="small" | ||
| bsStyle="primary" | ||
| style={{ width: '100px' }} | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. i know we haven't internationalized this yet, but in another language this button may need to be more than 100px wide. why is this width needed here? |
||
| onClick={this.runQuery.bind(this, true)} | ||
| disabled={!(this.props.queryEditor.dbId)} | ||
| > | ||
| <i className="fa fa-table" /> Run Async | ||
| </Button> | ||
| ); | ||
| } | ||
| runButtons = ( | ||
| <ButtonGroup bsSize="small" className="inline m-r-5 pull-left"> | ||
| {runButtons} | ||
| </ButtonGroup> | ||
| ); | ||
| if (this.props.latestQuery && this.props.latestQuery.state === 'running') { | ||
|
|
@@ -176,35 +192,6 @@ class SqlEditor extends React.Component { | |
| </ButtonGroup> | ||
| ); | ||
| } | ||
| const rightButtons = ( | ||
| <ButtonGroup className="inlineblock"> | ||
| <ButtonWithTooltip | ||
| tooltip="Save this query in your workspace" | ||
| placement="left" | ||
| bsSize="small" | ||
| onClick={this.addWorkspaceQuery.bind(this)} | ||
| > | ||
| <i className="fa fa-save" /> | ||
| </ButtonWithTooltip> | ||
| <DropdownButton | ||
| id="ddbtn-export" | ||
| pullRight | ||
| bsSize="small" | ||
| title={<i className="fa fa-file-o" />} | ||
| > | ||
| <MenuItem | ||
| onClick={this.notImplemented} | ||
| > | ||
| <i className="fa fa-file-text-o" /> export to .csv | ||
| </MenuItem> | ||
| <MenuItem | ||
| onClick={this.notImplemented} | ||
| > | ||
| <i className="fa fa-file-code-o" /> export to .json | ||
| </MenuItem> | ||
| </DropdownButton> | ||
| </ButtonGroup> | ||
| ); | ||
| let limitWarning = null; | ||
| const rowLimit = 1000; | ||
| if (this.props.latestQuery && this.props.latestQuery.rows === rowLimit) { | ||
|
|
@@ -256,7 +243,6 @@ class SqlEditor extends React.Component { | |
| <div className="pull-right"> | ||
| {limitWarning} | ||
| <Timer query={this.props.latestQuery} /> | ||
| {rightButtons} | ||
| </div> | ||
| </div> | ||
| ); | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -36,7 +36,7 @@ function addToObject(state, arrKey, obj) { | |
|
|
||
| function alterInObject(state, arrKey, obj, alterations) { | ||
| const newObject = Object.assign({}, state[arrKey]); | ||
| newObject[obj.id] = (Object.assign({}, newObject[obj.id], alterations)); | ||
| newObject[obj.id] = Object.assign({}, newObject[obj.id], alterations); | ||
| return Object.assign({}, state, { [arrKey]: newObject }); | ||
| } | ||
|
|
||
|
|
@@ -65,6 +65,16 @@ function removeFromArr(state, arrKey, obj, idKey = 'id') { | |
| return Object.assign({}, state, { [arrKey]: newArr }); | ||
| } | ||
|
|
||
| function getFromArr(arr, id) { | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this might be more clear as |
||
| let obj; | ||
| arr.forEach((o) => { | ||
| if (o.id === id) { | ||
| obj = o; | ||
| } | ||
| }); | ||
| return obj; | ||
| } | ||
|
|
||
| function addToArr(state, arrKey, obj) { | ||
| const newObj = Object.assign({}, obj); | ||
| if (!newObj.id) { | ||
|
|
@@ -87,9 +97,16 @@ export const sqlLabReducer = function (state, action) { | |
| let newState = removeFromArr(state, 'queryEditors', action.queryEditor); | ||
| // List of remaining queryEditor ids | ||
| const qeIds = newState.queryEditors.map((qe) => qe.id); | ||
| let th = state.tabHistory.slice(); | ||
| th = th.filter((id) => qeIds.includes(id)); | ||
| newState = Object.assign({}, newState, { tabHistory: th }); | ||
| const queries = {}; | ||
| Object.keys(state.queries).forEach((k) => { | ||
| const query = state.queries[k]; | ||
| if (qeIds.includes(query.sqlEditorId)) { | ||
| queries[k] = query; | ||
| } | ||
| }); | ||
| let tabHistory = state.tabHistory.slice(); | ||
| tabHistory = tabHistory.filter((id) => qeIds.includes(id)); | ||
| newState = Object.assign({}, newState, { tabHistory, queries }); | ||
| return newState; | ||
| }, | ||
| [actions.REMOVE_QUERY]() { | ||
|
|
@@ -113,20 +130,31 @@ export const sqlLabReducer = function (state, action) { | |
| return removeFromArr(state, 'tables', action.table); | ||
| }, | ||
| [actions.START_QUERY]() { | ||
| const newState = addToObject(state, 'queries', action.query); | ||
| const qe = getFromArr(state.queryEditors, action.query.sqlEditorId); | ||
| let newState = Object.assign({}, state); | ||
| if (qe.latestQueryId) { | ||
| const q = Object.assign({}, state.queries[qe.latestQueryId], { results: null }); | ||
| const queries = Object.assign({}, state.queries, { [q.id]: q }); | ||
| newState = Object.assign({}, state, { queries }); | ||
| } | ||
| newState = addToObject(newState, 'queries', action.query); | ||
| const sqlEditor = { id: action.query.sqlEditorId }; | ||
| return alterInArr(newState, 'queryEditors', sqlEditor, { latestQueryId: action.query.id }); | ||
| }, | ||
| [actions.STOP_QUERY]() { | ||
| return alterInObject(state, 'queries', action.query, { state: 'stopped' }); | ||
| }, | ||
| [actions.QUERY_SUCCESS]() { | ||
| let rows; | ||
| if (action.results.data) { | ||
| rows = action.results.data.length; | ||
| } | ||
| const alts = { | ||
| state: 'success', | ||
| results: action.results, | ||
| rows: action.results.data.length, | ||
| progress: 100, | ||
| endDttm: now(), | ||
| progress: 100, | ||
| results: action.results, | ||
| rows, | ||
| state: 'success', | ||
| }; | ||
| return alterInObject(state, 'queries', action.query, alts); | ||
| }, | ||
|
|
@@ -158,12 +186,6 @@ export const sqlLabReducer = function (state, action) { | |
| [actions.QUERY_EDITOR_SET_AUTORUN]() { | ||
| return alterInArr(state, 'queryEditors', action.queryEditor, { autorun: action.autorun }); | ||
| }, | ||
| [actions.ADD_WORKSPACE_QUERY]() { | ||
| return addToArr(state, 'workspaceQueries', action.query); | ||
| }, | ||
| [actions.REMOVE_WORKSPACE_QUERY]() { | ||
| return removeFromArr(state, 'workspaceQueries', action.query); | ||
| }, | ||
| [actions.ADD_ALERT]() { | ||
| return addToArr(state, 'alerts', action.alert); | ||
| }, | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -8,7 +8,6 @@ g.caravel path { | |
| } | ||
|
|
||
| text.nv-axislabel { | ||
| // font-weight: bold; | ||
| font-size: 14px; | ||
| } | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -206,6 +206,9 @@ class CeleryConfig(object): | |
| # The db id here results in selecting this one as a default in SQL Lab | ||
| DEFAULT_DB_ID = None | ||
|
|
||
| # Timeout duration for SQL Lab synchronous queries | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. s/Timeout duration for SQL Lab synchronous queries/Timeout duration for SQL Lab synchronous queries in seconds. |
||
| SQLLAB_TIMEOUT = 30 | ||
|
|
||
| try: | ||
| from caravel_config import * # noqa | ||
| except ImportError: | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,28 @@ | ||
| """allow_run_sync_async | ||
|
|
||
| Revision ID: 4500485bde7d | ||
| Revises: 41f6a59a61f2 | ||
| Create Date: 2016-09-12 23:33:14.789632 | ||
|
|
||
| """ | ||
|
|
||
| # revision identifiers, used by Alembic. | ||
| revision = '4500485bde7d' | ||
| down_revision = '41f6a59a61f2' | ||
|
|
||
| from alembic import op | ||
| import sqlalchemy as sa | ||
|
|
||
|
|
||
| def upgrade(): | ||
| op.add_column('dbs', sa.Column('allow_run_async', sa.Boolean(), nullable=True)) | ||
| op.add_column('dbs', sa.Column('allow_run_sync', sa.Boolean(), nullable=True)) | ||
|
|
||
|
|
||
| def downgrade(): | ||
| try: | ||
| op.drop_column('dbs', 'allow_run_sync') | ||
| op.drop_column('dbs', 'allow_run_async') | ||
| except Exception: | ||
| pass | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
could you please add the comment explaining the values in the tuple ?