Skip to content

Commit

Permalink
perf(dashboard): make loading datasets non-blocking (apache#15699)
Browse files Browse the repository at this point in the history
  • Loading branch information
ktmud authored Jul 15, 2021
1 parent c464b2d commit e9066bc
Show file tree
Hide file tree
Showing 12 changed files with 162 additions and 145 deletions.
2 changes: 1 addition & 1 deletion superset-frontend/src/chart/Chart.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ const propTypes = {
annotationData: PropTypes.object,
actions: PropTypes.object,
chartId: PropTypes.number.isRequired,
datasource: PropTypes.object.isRequired,
datasource: PropTypes.object,
// current chart is included by dashboard
dashboardId: PropTypes.number,
// original selected values for FilterBox viz
Expand Down
5 changes: 3 additions & 2 deletions superset-frontend/src/chart/ChartRenderer.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ const propTypes = {
annotationData: PropTypes.object,
actions: PropTypes.object,
chartId: PropTypes.number.isRequired,
datasource: PropTypes.object.isRequired,
datasource: PropTypes.object,
initialValues: PropTypes.object,
formData: PropTypes.object.isRequired,
height: PropTypes.number,
Expand Down Expand Up @@ -93,6 +93,7 @@ class ChartRenderer extends React.Component {
nextProps.queriesResponse !== this.props.queriesResponse;
return (
this.hasQueryResponseChange ||
nextProps.datasource !== this.props.datasource ||
nextProps.annotationData !== this.props.annotationData ||
nextProps.ownState !== this.props.ownState ||
nextProps.filterState !== this.props.filterState ||
Expand Down Expand Up @@ -223,7 +224,7 @@ class ChartRenderer extends React.Component {
width={width}
height={height}
annotationData={annotationData}
datasource={datasource}
datasource={datasource || {}}
initialValues={initialValues}
formData={formData}
ownState={ownState}
Expand Down
7 changes: 4 additions & 3 deletions superset-frontend/src/common/hooks/apiResources/dashboards.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,8 @@
* under the License.
*/

import Dashboard from 'src/types/Dashboard';
import { Dashboard, Datasource } from 'src/dashboard/types';
import { Chart } from 'src/types/Chart';
import { useApiV1Resource, useTransformedResource } from './apiResources';

export const useDashboard = (idOrSlug: string | number) =>
Expand All @@ -33,10 +34,10 @@ export const useDashboard = (idOrSlug: string | number) =>

// gets the chart definitions for a dashboard
export const useDashboardCharts = (idOrSlug: string | number) =>
useApiV1Resource(`/api/v1/dashboard/${idOrSlug}/charts`);
useApiV1Resource<Chart[]>(`/api/v1/dashboard/${idOrSlug}/charts`);

// gets the datasets for a dashboard
// important: this endpoint only returns the fields in the dataset
// that are necessary for rendering the given dashboard
export const useDashboardDatasets = (idOrSlug: string | number) =>
useApiV1Resource(`/api/v1/dashboard/${idOrSlug}/datasets`);
useApiV1Resource<Datasource[]>(`/api/v1/dashboard/${idOrSlug}/datasets`);
Original file line number Diff line number Diff line change
Expand Up @@ -16,26 +16,44 @@
* specific language governing permissions and limitations
* under the License.
*/
import { Dispatch } from 'redux';
import { SupersetClient } from '@superset-ui/core';
import { getClientErrorObject } from '../../utils/getClientErrorObject';
import { Datasource, RootState } from 'src/dashboard/types';

export const SET_DATASOURCE = 'SET_DATASOURCE';
export function setDatasource(datasource, key) {
return { type: SET_DATASOURCE, datasource, key };
// update datasources index for Dashboard
export enum DatasourcesAction {
SET_DATASOURCES = 'SET_DATASOURCES',
SET_DATASOURCE = 'SET_DATASOURCE',
}

export const FETCH_DATASOURCE_STARTED = 'FETCH_DATASOURCE_STARTED';
export function fetchDatasourceStarted(key) {
return { type: FETCH_DATASOURCE_STARTED, key };
export type DatasourcesActionPayload =
| {
type: DatasourcesAction.SET_DATASOURCES;
datasources: Datasource[] | null;
}
| {
type: DatasourcesAction.SET_DATASOURCE;
key: Datasource['uid'];
datasource: Datasource;
};

export function setDatasources(datasources: Datasource[] | null) {
return {
type: DatasourcesAction.SET_DATASOURCES,
datasources,
};
}

export const FETCH_DATASOURCE_FAILED = 'FETCH_DATASOURCE_FAILED';
export function fetchDatasourceFailed(error, key) {
return { type: FETCH_DATASOURCE_FAILED, error, key };
export function setDatasource(datasource: Datasource, key: string) {
return {
type: DatasourcesAction.SET_DATASOURCE,
key,
datasource,
};
}

export function fetchDatasourceMetadata(key) {
return (dispatch, getState) => {
export function fetchDatasourceMetadata(key: string) {
return (dispatch: Dispatch, getState: () => RootState) => {
const { datasources } = getState();
const datasource = datasources[key];

Expand All @@ -45,12 +63,6 @@ export function fetchDatasourceMetadata(key) {

return SupersetClient.get({
endpoint: `/superset/fetch_datasource_metadata?datasourceKey=${key}`,
})
.then(({ json }) => dispatch(setDatasource(json, key)))
.catch(response =>
getClientErrorObject(response).then(({ error }) =>
dispatch(fetchDatasourceFailed(error, key)),
),
);
}).then(({ json }) => dispatch(setDatasource(json as Datasource, key)));
};
}
5 changes: 2 additions & 3 deletions superset-frontend/src/dashboard/actions/hydrate.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
* under the License.
*/
/* eslint-disable camelcase */
import { isString, keyBy } from 'lodash';
import { isString } from 'lodash';
import {
Behavior,
CategoricalColorNamespace,
Expand Down Expand Up @@ -60,7 +60,7 @@ import extractUrlParams from '../util/extractUrlParams';

export const HYDRATE_DASHBOARD = 'HYDRATE_DASHBOARD';

export const hydrateDashboard = (dashboardData, chartData, datasourcesData) => (
export const hydrateDashboard = (dashboardData, chartData) => (
dispatch,
getState,
) => {
Expand Down Expand Up @@ -329,7 +329,6 @@ export const hydrateDashboard = (dashboardData, chartData, datasourcesData) => (
return dispatch({
type: HYDRATE_DASHBOARD,
data: {
datasources: keyBy(datasourcesData, 'uid'),
sliceEntities: { ...initSliceEntities, slices, isLoading: false },
charts: chartQueries,
// read-only data
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ const propTypes = {
// from redux
chart: chartPropShape.isRequired,
formData: PropTypes.object.isRequired,
datasource: PropTypes.object.isRequired,
datasource: PropTypes.object,
slice: slicePropShape.isRequired,
sliceName: PropTypes.string.isRequired,
timeout: PropTypes.number.isRequired,
Expand Down Expand Up @@ -125,7 +125,8 @@ export default class Chart extends React.Component {
if (
nextState.width !== this.state.width ||
nextState.height !== this.state.height ||
nextState.descriptionHeight !== this.state.descriptionHeight
nextState.descriptionHeight !== this.state.descriptionHeight ||
nextProps.datasource !== this.props.datasource
) {
return true;
}
Expand Down
17 changes: 10 additions & 7 deletions superset-frontend/src/dashboard/containers/Dashboard.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,19 +18,21 @@
*/
import { bindActionCreators, Dispatch } from 'redux';
import { connect } from 'react-redux';
import Dashboard from '../components/Dashboard';
import { RootState } from 'src/dashboard/types';
import Dashboard from 'src/dashboard/components/Dashboard';
import {
addSliceToDashboard,
removeSliceFromDashboard,
} from '../actions/dashboardState';
import { triggerQuery } from '../../chart/chartAction';
import { logEvent } from '../../logger/actions';
import { getActiveFilters } from '../util/activeDashboardFilters';
} from 'src/dashboard/actions/dashboardState';
import { setDatasources } from 'src/dashboard/actions/datasources';

import { triggerQuery } from 'src/chart/chartAction';
import { logEvent } from 'src/logger/actions';
import { getActiveFilters } from 'src/dashboard/util/activeDashboardFilters';
import {
getAllActiveFilters,
getRelevantDataMask,
} from '../util/activeAllDashboardFilters';
import { RootState } from '../types';
} from 'src/dashboard/util/activeAllDashboardFilters';

function mapStateToProps(state: RootState) {
const {
Expand Down Expand Up @@ -80,6 +82,7 @@ function mapDispatchToProps(dispatch: Dispatch) {
return {
actions: bindActionCreators(
{
setDatasources,
addSliceToDashboard,
removeSliceFromDashboard,
triggerQuery,
Expand Down
77 changes: 44 additions & 33 deletions superset-frontend/src/dashboard/containers/DashboardPage.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -16,17 +16,19 @@
* specific language governing permissions and limitations
* under the License.
*/
import React, { useEffect, useState, FC } from 'react';
import React, { useEffect, FC } from 'react';
import { t } from '@superset-ui/core';
import { useDispatch } from 'react-redux';
import { useParams } from 'react-router-dom';
import { useToasts } from 'src/messageToasts/enhancers/withToasts';
import Loading from 'src/components/Loading';
import {
useDashboard,
useDashboardCharts,
useDashboardDatasets,
} from 'src/common/hooks/apiResources';
import { ResourceStatus } from 'src/common/hooks/apiResources/apiResources';
import { hydrateDashboard } from 'src/dashboard/actions/hydrate';
import { setDatasources } from 'src/dashboard/actions/datasources';
import injectCustomCss from 'src/dashboard/util/injectCustomCss';

const DashboardContainer = React.lazy(
Expand All @@ -40,48 +42,57 @@ const DashboardContainer = React.lazy(

const DashboardPage: FC = () => {
const dispatch = useDispatch();
const { addDangerToast } = useToasts();
const { idOrSlug } = useParams<{ idOrSlug: string }>();
const [isHydrated, setHydrated] = useState(false);
const dashboardResource = useDashboard(idOrSlug);
const chartsResource = useDashboardCharts(idOrSlug);
const datasetsResource = useDashboardDatasets(idOrSlug);
const error = [dashboardResource, chartsResource, datasetsResource].find(
resource => resource.status === ResourceStatus.ERROR,
)?.error;
const { result: dashboard, error: dashboardApiError } = useDashboard(
idOrSlug,
);
const { result: charts, error: chartsApiError } = useDashboardCharts(
idOrSlug,
);
const { result: datasets, error: datasetsApiError } = useDashboardDatasets(
idOrSlug,
);

const error = dashboardApiError || chartsApiError;
const readyToRender = Boolean(dashboard && charts);
const { dashboard_title, css } = dashboard || {};

useEffect(() => {
if (dashboardResource.result) {
document.title = dashboardResource.result.dashboard_title;
if (dashboardResource.result.css) {
// returning will clean up custom css
// when dashboard unmounts or changes
return injectCustomCss(dashboardResource.result.css);
}
if (readyToRender) {
dispatch(hydrateDashboard(dashboard, charts));
}
return () => {};
}, [dashboardResource.result]);
// eslint-disable-next-line react-hooks/exhaustive-deps
}, [readyToRender]);

const shouldBeHydrated =
dashboardResource.status === ResourceStatus.COMPLETE &&
chartsResource.status === ResourceStatus.COMPLETE &&
datasetsResource.status === ResourceStatus.COMPLETE;
useEffect(() => {
if (dashboard_title) {
document.title = dashboard_title;
}
}, [dashboard_title]);

useEffect(() => {
if (shouldBeHydrated) {
dispatch(
hydrateDashboard(
dashboardResource.result,
chartsResource.result,
datasetsResource.result,
),
if (css) {
// returning will clean up custom css
// when dashboard unmounts or changes
return injectCustomCss(css);
}
return () => {};
}, [css]);

useEffect(() => {
if (datasetsApiError) {
addDangerToast(
t('Error loading chart datasources. Filters may not work correctly.'),
);
setHydrated(true);
} else {
dispatch(setDatasources(datasets));
}
// eslint-disable-next-line react-hooks/exhaustive-deps
}, [shouldBeHydrated]);
}, [addDangerToast, datasets, datasetsApiError, dispatch]);

if (error) throw error; // caught in error boundary
if (!isHydrated) return <Loading />;
if (!readyToRender) return <Loading />;

return <DashboardContainer />;
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,30 +16,28 @@
* specific language governing permissions and limitations
* under the License.
*/
import { SET_DATASOURCE } from '../actions/datasources';
import { HYDRATE_DASHBOARD } from '../actions/hydrate';
import { keyBy } from 'lodash';
import { DatasourcesState } from 'src/dashboard/types';
import {
DatasourcesActionPayload,
DatasourcesAction,
} from '../actions/datasources';

export default function datasourceReducer(datasources = {}, action) {
const actionHandlers = {
[HYDRATE_DASHBOARD]() {
return action.data.datasources;
},
[SET_DATASOURCE]() {
return action.datasource;
},
};

if (action.type in actionHandlers) {
if (action.key) {
return {
...datasources,
[action.key]: actionHandlers[action.type](
datasources[action.key],
action,
),
};
}
return actionHandlers[action.type]();
export default function datasourcesReducer(
datasources: DatasourcesState | undefined,
action: DatasourcesActionPayload,
) {
if (action.type === DatasourcesAction.SET_DATASOURCES) {
return {
...datasources,
...keyBy(action.datasources, 'uid'),
};
}
if (action.type === DatasourcesAction.SET_DATASOURCE) {
return {
...datasources,
[action.key]: action.datasource,
};
}
return datasources;
return datasources || {};
}
Loading

0 comments on commit e9066bc

Please sign in to comment.