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

Full Annotation Framework #3518

Merged
merged 8 commits into from
Dec 17, 2017
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
1,135 changes: 869 additions & 266 deletions superset/assets/backendSync.json

Large diffs are not rendered by default.

5 changes: 4 additions & 1 deletion superset/assets/javascripts/chart/Chart.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import StackTraceMessage from '../components/StackTraceMessage';
import visMap from '../../visualizations/main';

const propTypes = {
annotationData: PropTypes.object,
actions: PropTypes.object,
chartKey: PropTypes.string.isRequired,
containerId: PropTypes.string.isRequired,
Expand Down Expand Up @@ -47,8 +48,8 @@ const defaultProps = {
class Chart extends React.PureComponent {
constructor(props) {
super(props);

// these properties are used by visualizations
this.annotationData = props.annotationData;
this.containerId = props.containerId;
this.selector = `#${this.containerId}`;
this.formData = props.formData;
Expand All @@ -71,6 +72,7 @@ class Chart extends React.PureComponent {
}

componentWillReceiveProps(nextProps) {
this.annotationData = nextProps.annotationData;
this.containerId = nextProps.containerId;
this.selector = `#${this.containerId}`;
this.formData = nextProps.formData;
Expand All @@ -82,6 +84,7 @@ class Chart extends React.PureComponent {
this.props.queryResponse &&
this.props.chartStatus === 'success' &&
!this.props.queryResponse.error && (
prevProps.annotationData !== this.props.annotationData ||
prevProps.queryResponse !== this.props.queryResponse ||
prevProps.height !== this.props.height ||
prevProps.width !== this.props.width ||
Expand Down
1 change: 1 addition & 0 deletions superset/assets/javascripts/chart/ChartContainer.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import Chart from './Chart';
function mapStateToProps({ charts }, ownProps) {
const chart = charts[ownProps.chartKey];
return {
annotationData: chart.annotationData,
chartAlert: chart.chartAlert,
chartStatus: chart.chartStatus,
chartUpdateEndTime: chart.chartUpdateEndTime,
Expand Down
96 changes: 69 additions & 27 deletions superset/assets/javascripts/chart/chartAction.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { getExploreUrl } from '../explore/exploreUtils';
import { t } from '../locales';
import { getExploreUrl, getAnnotationJsonUrl } from '../explore/exploreUtils';
import { requiresQuery, ANNOTATION_SOURCE_TYPES } from '../modules/AnnotationTypes';

const $ = window.$ = require('jquery');

Expand Down Expand Up @@ -41,6 +41,57 @@ export function removeChart(key) {
return { type: REMOVE_CHART, key };
}

export const ANNOTATION_QUERY_SUCCESS = 'ANNOTATION_QUERY_SUCCESS';
export function annotationQuerySuccess(annotation, queryResponse, key) {
return { type: ANNOTATION_QUERY_SUCCESS, annotation, queryResponse, key };
}

export const ANNOTATION_QUERY_STARTED = 'ANNOTATION_QUERY_STARTED';
export function annotationQueryStarted(annotation, queryRequest, key) {
return { type: ANNOTATION_QUERY_STARTED, annotation, queryRequest, key };
}

export const ANNOTATION_QUERY_FAILED = 'ANNOTATION_QUERY_FAILED';
export function annotationQueryFailed(annotation, queryResponse, key) {
return { type: ANNOTATION_QUERY_FAILED, annotation, queryResponse, key };
}

export function runAnnotationQuery(annotation, timeout = 60, formData = null, key) {
return function (dispatch, getState) {
const sliceKey = key || Object.keys(getState().charts)[0];
const fd = formData || getState().charts[sliceKey].latestQueryFormData;

if (!requiresQuery(annotation.sourceType)) {
return Promise.resolve();
}

const sliceFormData = Object.keys(annotation.overrides)
.reduce((d, k) => ({
...d,
[k]: annotation.overrides[k] || fd[k],
}), {});
const isNative = annotation.sourceType === ANNOTATION_SOURCE_TYPES.NATIVE;
const url = getAnnotationJsonUrl(annotation.value, sliceFormData, isNative);
const queryRequest = $.ajax({
url,
dataType: 'json',
timeout: timeout * 1000,
});
dispatch(annotationQueryStarted(annotation, queryRequest, sliceKey));
return queryRequest
.then(queryResponse => dispatch(annotationQuerySuccess(annotation, queryResponse, sliceKey)))
.catch((err) => {
if (err.statusText === 'timeout') {
dispatch(annotationQueryFailed(annotation, { error: 'Query Timeout' }, sliceKey));
} else if ((err.responseJSON.error || '').toLowerCase().startsWith('no data')) {
dispatch(annotationQuerySuccess(annotation, err, sliceKey));
} else if (err.statusText !== 'abort') {
dispatch(annotationQueryFailed(annotation, err.responseJSON, sliceKey));
}
});
};
}

export const TRIGGER_QUERY = 'TRIGGER_QUERY';
export function triggerQuery(value = true, key) {
return { type: TRIGGER_QUERY, value, key };
Expand All @@ -60,32 +111,23 @@ export function runQuery(formData, force = false, timeout = 60, key) {
url,
dataType: 'json',
timeout: timeout * 1000,
success: (queryResponse =>
dispatch(chartUpdateSucceeded(queryResponse, key))
),
error: ((xhr) => {
if (xhr.statusText === 'timeout') {
dispatch(chartUpdateTimeout(xhr.statusText, timeout, key));
} else {
let error = '';
if (!xhr.responseText) {
const status = xhr.status;
if (status === 0) {
// This may happen when the worker in gunicorn times out
error += (
t('The server could not be reached. You may want to ' +
'verify your connection and try again.'));
} else {
error += (t('An unknown error occurred. (Status: %s )', status));
}
}
const errorResponse = Object.assign({}, xhr.responseJSON, error);
dispatch(chartUpdateFailed(errorResponse, key));
}
}),
});

dispatch(chartUpdateStarted(queryRequest, key));
dispatch(triggerQuery(false, key));
const queryPromise = Promise.resolve(dispatch(chartUpdateStarted(queryRequest, key)))
.then(() => queryRequest)
.then(queryResponse => dispatch(chartUpdateSucceeded(queryResponse, key)))
.catch((err) => {
if (err.statusText === 'timeout') {
dispatch(chartUpdateTimeout(err.statusText, timeout, key));
} else if (err.statusText !== 'abort') {
dispatch(chartUpdateFailed(err.responseJSON, key));
}
});
const annotationLayers = formData.annotation_layers || [];
return Promise.all([
queryPromise,
dispatch(triggerQuery(false, key)),
...annotationLayers.map(x => dispatch(runAnnotationQuery(x, timeout, formData, key))),
]);
};
}
59 changes: 53 additions & 6 deletions superset/assets/javascripts/chart/chartReducer.js
Original file line number Diff line number Diff line change
Expand Up @@ -65,12 +65,12 @@ export default function chartReducer(charts = {}, action) {
return { ...state,
chartStatus: 'failed',
chartAlert: (
`<strong>${t('Query timeout')}</strong> - ` +
t(`visualization queries are set to timeout at ${action.timeout} seconds. `) +
t('Perhaps your data has grown, your database is under unusual load, ' +
'or you are simply querying a data source that is too large ' +
'to be processed within the timeout range. ' +
'If that is the case, we recommend that you summarize your data further.')),
`<strong>${t('Query timeout')}</strong> - ` +
t(`visualization queries are set to timeout at ${action.timeout} seconds. `) +
t('Perhaps your data has grown, your database is under unusual load, ' +
'or you are simply querying a data source that is too large ' +
'to be processed within the timeout range. ' +
'If that is the case, we recommend that you summarize your data further.')),
};
},
[actions.CHART_UPDATE_FAILED](state) {
Expand All @@ -87,6 +87,53 @@ export default function chartReducer(charts = {}, action) {
[actions.RENDER_TRIGGERED](state) {
return { ...state, lastRendered: action.value };
},
[actions.ANNOTATION_QUERY_STARTED](state) {
if (state.annotationQuery &&
state.annotationQuery[action.annotation.name]) {
state.annotationQuery[action.annotation.name].abort();
}
const annotationQuery = {
...state.annotationQuery,
[action.annotation.name]: action.queryRequest,
};
return {
...state,
annotationQuery,
};
},
[actions.ANNOTATION_QUERY_SUCCESS](state) {
const annotationData = {
...state.annotationData,
[action.annotation.name]: action.queryResponse.data,
Copy link
Member

@mistercrunch mistercrunch Dec 16, 2017

Choose a reason for hiding this comment

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

I wonder if we should use a shortid like we do in other places in the code. Otherwise we have to make sure names are unique as they get inputed

};
const annotationError = { ...state.annotationError };
delete annotationError[action.annotation.name];
const annotationQuery = { ...state.annotationQuery };
delete annotationQuery[action.annotation.name];
return {
...state,
annotationData,
annotationError,
annotationQuery,
};
},
[actions.ANNOTATION_QUERY_FAILED](state) {
const annotationData = { ...state.annotationData };
delete annotationData[action.annotation.name];
const annotationError = {
...state.annotationError,
[action.annotation.name]: action.queryResponse ?
action.queryResponse.error : t('Network error.'),
};
const annotationQuery = { ...state.annotationQuery };
delete annotationQuery[action.annotation.name];
return {
...state,
annotationData,
annotationError,
annotationQuery,
};
},
};

/* eslint-disable no-param-reassign */
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ const propTypes = {
clearFilter: PropTypes.func,
removeFilter: PropTypes.func,
editMode: PropTypes.bool,
annotationQuery: PropTypes.object,
};

const defaultProps = {
Expand Down Expand Up @@ -84,7 +85,7 @@ class GridCell extends React.PureComponent {
const {
exploreChartUrl, exportCSVUrl, isExpanded, isLoading, isCached, cachedDttm,
removeSlice, updateSliceName, toggleExpandSlice, forceRefresh,
chartKey, slice, datasource, formData, timeout,
chartKey, slice, datasource, formData, timeout, annotationQuery,
} = this.props;
return (
<div
Expand All @@ -104,6 +105,7 @@ class GridCell extends React.PureComponent {
toggleExpandSlice={toggleExpandSlice}
forceRefresh={forceRefresh}
editMode={this.props.editMode}
annotationQuery={annotationQuery}
/>
</div>
<div
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -164,6 +164,8 @@ class GridLayout extends React.Component {
clearFilter={this.props.clearFilter}
removeFilter={this.props.removeFilter}
editMode={this.props.editMode}
annotationQuery={currentChart.annotationQuery}
annotationError={currentChart.annotationError}
/>
</div>);
});
Expand Down
22 changes: 22 additions & 0 deletions superset/assets/javascripts/dashboard/components/SliceHeader.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@ const propTypes = {
toggleExpandSlice: PropTypes.func,
forceRefresh: PropTypes.func,
editMode: PropTypes.bool,
annotationQuery: PropTypes.object,
annotationError: PropTypes.object,
};

const defaultProps = {
Expand Down Expand Up @@ -50,6 +52,8 @@ class SliceHeader extends React.PureComponent {
const refreshTooltip = isCached ?
t('Served from data cached %s . Click to force refresh.', cachedWhen) :
t('Force refresh data');
const annoationsLoading = t('Annotation layers are still loading.');
const annoationsError = t('One ore more annotation layers failed loading.');

return (
<div className="row chart-header">
Expand All @@ -61,6 +65,24 @@ class SliceHeader extends React.PureComponent {
onSaveTitle={this.onSaveTitle}
noPermitTooltip={'You don\'t have the rights to alter this dashboard.'}
/>
{!!Object.values(this.props.annotationQuery || {}).length &&
<TooltipWrapper
label="annotations-loading"
placement="top"
tooltip={annoationsLoading}
>
<i className="fa fa-refresh warning" />
</TooltipWrapper>
}
{!!Object.values(this.props.annotationError || {}).length &&
<TooltipWrapper
label="annoation-errors"
placement="top"
tooltip={annoationsError}
>
<i className="fa fa-exclamation-circle danger" />
</TooltipWrapper>
}
</div>
<div className="chart-controls">
<div id={'controls_' + slice.slice_id} className="pull-right">
Expand Down
Loading