Explore: Remove storing derived "loading" property (#70324)

* Remove storing derived state (loading property) and use a selector instead

* Remove redundant tests

There's no way to change the interval while live streaming

* Remove check for isLive when deriving waiting for data

It was introduced in #18804

* Remove unused props
This commit is contained in:
Piotr Jamróz
2023-06-20 17:38:05 +02:00
committed by GitHub
parent 47b2e7f1d7
commit a94ba7784a
15 changed files with 46 additions and 89 deletions

View File

@@ -3,7 +3,7 @@ import React from 'react';
import AutoSizer from 'react-virtualized-auto-sizer';
import { GrafanaTheme2, isTimeSeriesFrames, PanelData, ThresholdsConfig } from '@grafana/data';
import { GraphTresholdsStyleMode, LoadingState } from '@grafana/schema';
import { GraphTresholdsStyleMode } from '@grafana/schema';
import { useStyles2 } from '@grafana/ui';
import appEvents from 'app/core/app_events';
import { GraphContainer } from 'app/features/explore/Graph/GraphContainer';
@@ -37,7 +37,6 @@ export const VizWrapper = ({ data, thresholds, thresholdsType }: Props) => {
<div style={{ width }}>
{isTimeSeriesData ? (
<GraphContainer
loading={data.state === LoadingState.Loading}
statusMessage={statusMessage}
data={data.series}
eventBus={appEvents}

View File

@@ -61,6 +61,7 @@ import {
modifyQueries,
scanStart,
scanStopAction,
selectIsWaitingForData,
setQueries,
setSupplementaryQueryEnabled,
} from './state/query';
@@ -284,11 +285,10 @@ export class Explore extends React.PureComponent<Props, ExploreState> {
}
renderGraphPanel(width: number) {
const { graphResult, absoluteRange, timeZone, queryResponse, loading, showFlameGraph } = this.props;
const { graphResult, absoluteRange, timeZone, queryResponse, showFlameGraph } = this.props;
return (
<GraphContainer
loading={loading}
data={graphResult!}
height={showFlameGraph ? 180 : 400}
width={width}
@@ -551,11 +551,11 @@ function mapStateToProps(state: StoreState, { exploreId }: ExploreProps) {
queryResponse,
showNodeGraph,
showFlameGraph,
loading,
showRawPrometheus,
supplementaryQueries,
} = item;
const loading = selectIsWaitingForData(exploreId)(state);
const logsSample = supplementaryQueries[SupplementaryQueryType.LogsSample];
// We want to show logs sample only if there are no log results and if there is already graph or table result
const showLogsSample = !!(logsSample.dataProvider !== undefined && !logsResult && (graphResult || tableResult));

View File

@@ -12,7 +12,7 @@ import { InspectStatsTab } from 'app/features/inspector/InspectStatsTab';
import { QueryInspector } from 'app/features/inspector/QueryInspector';
import { StoreState, ExploreItemState, ExploreId } from 'app/types';
import { runQueries } from './state/query';
import { runQueries, selectIsWaitingForData } from './state/query';
interface DispatchProps {
width: number;
@@ -93,10 +93,10 @@ export function ExploreQueryInspector(props: Props) {
function mapStateToProps(state: StoreState, { exploreId }: { exploreId: ExploreId }) {
const explore = state.explore;
const item: ExploreItemState = explore.panes[exploreId]!;
const { loading, queryResponse } = item;
const { queryResponse } = item;
return {
loading,
loading: selectIsWaitingForData(exploreId)(state),
queryResponse,
};
}

View File

@@ -23,7 +23,7 @@ import { ExploreTimeControls } from './ExploreTimeControls';
import { LiveTailButton } from './LiveTailButton';
import { changeDatasource } from './state/datasource';
import { splitClose, splitOpen, maximizePaneAction, evenPaneResizeAction } from './state/main';
import { cancelQueries, runQueries } from './state/query';
import { cancelQueries, runQueries, selectIsWaitingForData } from './state/query';
import { isSplit } from './state/selectors';
import { syncTimes, changeRefreshInterval } from './state/time';
import { LiveTailControls } from './useLiveTailControls';
@@ -50,21 +50,14 @@ export function ExploreToolbar({ exploreId, topOfViewRef, onChangeTime }: Props)
const splitted = useSelector(isSplit);
const timeZone = useSelector((state: StoreState) => getTimeZone(state.user));
const fiscalYearStartMonth = useSelector((state: StoreState) => getFiscalYearStartMonth(state.user));
const { refreshInterval, loading, datasourceInstance, range, isLive, isPaused, syncedTimes } = useSelector(
const { refreshInterval, datasourceInstance, range, isLive, isPaused, syncedTimes } = useSelector(
(state: StoreState) => ({
...pick(
state.explore.panes[exploreId]!,
'refreshInterval',
'loading',
'datasourceInstance',
'range',
'isLive',
'isPaused'
),
...pick(state.explore.panes[exploreId]!, 'refreshInterval', 'datasourceInstance', 'range', 'isLive', 'isPaused'),
syncedTimes: state.explore.syncedTimes,
}),
shallowEqual
);
const loading = useSelector(selectIsWaitingForData(exploreId));
const isLargerPane = useSelector((state: StoreState) => state.explore.largerExploreId === exploreId);
const showSmallTimePicker = useSelector((state) => splitted || state.explore.panes[exploreId]!.containerWidth < 1210);
const showSmallDataSourcePicker = useSelector(

View File

@@ -19,7 +19,6 @@ import { ExploreGraphLabel } from './ExploreGraphLabel';
import { loadGraphStyle } from './utils';
interface Props extends Pick<PanelChromeProps, 'width' | 'height' | 'statusMessage'> {
loading: boolean;
data: DataFrame[];
annotations?: DataFrame[];
eventBus: EventBus;

View File

@@ -28,6 +28,7 @@ import {
addResultsToCache,
clearCache,
loadSupplementaryQueryData,
selectIsWaitingForData,
setSupplementaryQueryEnabled,
} from '../state/query';
import { updateTimeRange } from '../state/time';
@@ -222,7 +223,6 @@ function mapStateToProps(state: StoreState, { exploreId }: { exploreId: ExploreI
const item: ExploreItemState = explore.panes[exploreId]!;
const {
logsResult,
loading,
scanning,
datasourceInstance,
isLive,
@@ -232,6 +232,7 @@ function mapStateToProps(state: StoreState, { exploreId }: { exploreId: ExploreI
absoluteRange,
supplementaryQueries,
} = item;
const loading = selectIsWaitingForData(exploreId)(state);
const panelState = item.panelsState;
const timeZone = getTimeZone(state.user);
const logsVolume = supplementaryQueries[SupplementaryQueryType.LogsVolume];

View File

@@ -12,6 +12,7 @@ import { ExploreId, ExploreItemState, TABLE_RESULTS_STYLES, TableResultsStyle }
import { MetaInfoText } from '../MetaInfoText';
import RawListContainer from '../PrometheusListView/RawListContainer';
import { selectIsWaitingForData } from '../state/query';
import { getFieldLinksForExplore } from '../utils/links';
interface RawPrometheusContainerProps {
@@ -31,7 +32,8 @@ interface PrometheusContainerState {
function mapStateToProps(state: StoreState, { exploreId }: RawPrometheusContainerProps) {
const explore = state.explore;
const item: ExploreItemState = explore.panes[exploreId]!;
const { loading: loadingInState, tableResult, rawPrometheusResult, range } = item;
const { tableResult, rawPrometheusResult, range } = item;
const loadingInState = selectIsWaitingForData(exploreId)(state);
const rawPrometheusFrame: DataFrame[] = rawPrometheusResult ? [rawPrometheusResult] : [];
const result = (tableResult?.length ?? false) > 0 && rawPrometheusResult ? tableResult : rawPrometheusFrame;
const loading = result && result.length > 0 ? false : loadingInState;

View File

@@ -8,6 +8,7 @@ import { StoreState } from 'app/types';
import { ExploreId, ExploreItemState } from 'app/types/explore';
import { MetaInfoText } from '../MetaInfoText';
import { selectIsWaitingForData } from '../state/query';
import { getFieldLinksForExplore } from '../utils/links';
interface TableContainerProps {
@@ -22,7 +23,8 @@ interface TableContainerProps {
function mapStateToProps(state: StoreState, { exploreId }: TableContainerProps) {
const explore = state.explore;
const item: ExploreItemState = explore.panes[exploreId]!;
const { loading: loadingInState, tableResult, range } = item;
const { tableResult, range } = item;
const loadingInState = selectIsWaitingForData(exploreId);
const loading = tableResult && tableResult.length > 0 ? false : loadingInState;
return { loading, tableResult, range };
}

View File

@@ -37,7 +37,6 @@ describe('Datasource reducer', () => {
graphResult: null,
logsResult: null,
tableResult: null,
loading: false,
queryResponse: {
// When creating an empty query response we also create a timeRange object with the current time.
// Copying the range from the reducer here prevents intermittent failures when creating them at different times.

View File

@@ -104,7 +104,6 @@ export const datasourceReducer = (state: ExploreItemState, action: AnyAction): E
logsResult: null,
supplementaryQueries: loadSupplementaryQueries(),
queryResponse: createEmptyQueryResponse(),
loading: false,
queryKeys: [],
history,
};

View File

@@ -43,6 +43,7 @@ import {
ExploreItemState,
ExplorePanelData,
QueryTransaction,
StoreState,
ThunkDispatch,
ThunkResult,
} from 'app/types';
@@ -53,14 +54,30 @@ import { createErrorNotification } from '../../../core/copy/appNotification';
import { runRequest } from '../../query/state/runRequest';
import { decorateData } from '../utils/decorators';
import {
getSupplementaryQueryProvider,
storeSupplementaryQueryEnabled,
supplementaryQueryTypes,
getSupplementaryQueryProvider,
} from '../utils/supplementaryQueries';
import { addHistoryItem, historyUpdatedAction, loadRichHistory } from './history';
import { updateTime } from './time';
import { createCacheKey, getResultsFromCache, filterLogRowsByIndex } from './utils';
import { createCacheKey, filterLogRowsByIndex, getResultsFromCache } from './utils';
/**
* Derives from explore state if a given Explore pane is waiting for more data to be received
*/
export const selectIsWaitingForData = (exploreId: ExploreId) => {
return (state: StoreState) => {
const panelState = state.explore.panes[exploreId];
if (!panelState) {
return false;
}
return panelState.queryResponse
? panelState.queryResponse.state === LoadingState.Loading ||
panelState.queryResponse.state === LoadingState.Streaming
: false;
};
};
/**
* Adds a query row after the row with the given index.
@@ -878,7 +895,10 @@ export const queryReducer = (state: ExploreItemState, action: AnyAction): Explor
return {
...state,
loading: false,
queryResponse: {
...state.queryResponse,
state: LoadingState.Done,
},
};
}
@@ -1024,7 +1044,6 @@ export const queryReducer = (state: ExploreItemState, action: AnyAction): Explor
...state.queryResponse,
state: loadingState,
},
loading: loadingState === LoadingState.Loading || loadingState === LoadingState.Streaming,
};
}
@@ -1140,7 +1159,6 @@ export const processQueryResponse = (
const { response } = action.payload;
const {
request,
state: loadingState,
series,
error,
graphResult,
@@ -1154,14 +1172,8 @@ export const processQueryResponse = (
} = response;
if (error) {
if (error.type === DataQueryErrorType.Timeout) {
return {
...state,
queryResponse: response,
loading: loadingState === LoadingState.Loading || loadingState === LoadingState.Streaming,
};
} else if (error.type === DataQueryErrorType.Cancelled) {
return state;
if (error.type === DataQueryErrorType.Timeout || error.type === DataQueryErrorType.Cancelled) {
return { ...state };
}
// Send error to Angular editors
@@ -1192,7 +1204,6 @@ export const processQueryResponse = (
state.isLive && logsResult
? { ...logsResult, rows: filterLogRowsByIndex(state.clearedAtIndex, logsResult.rows) }
: logsResult,
loading: loadingState === LoadingState.Loading || loadingState === LoadingState.Streaming,
showLogs: !!logsResult,
showMetrics: !!graphResult,
showTable: !!tableResult?.length,

View File

@@ -1,12 +1,11 @@
import { reducerTester } from 'test/core/redux/reducerTester';
import { dateTime, LoadingState } from '@grafana/data';
import { dateTime } from '@grafana/data';
import { configureStore } from 'app/store/configureStore';
import { ExploreId, ExploreItemState } from 'app/types';
import { createDefaultInitialState } from './helpers';
import { changeRangeAction, changeRefreshInterval, timeReducer, updateTime } from './time';
import { makeExplorePaneState } from './utils';
import { changeRangeAction, timeReducer, updateTime } from './time';
const MOCK_TIME_RANGE = {};
@@ -37,50 +36,6 @@ describe('Explore item reducer', () => {
});
});
describe('changing refresh intervals', () => {
it("should result in 'streaming' state, when live-tailing is active", () => {
const initialState = makeExplorePaneState();
const expectedState = {
...initialState,
refreshInterval: 'LIVE',
isLive: true,
loading: true,
logsResult: {
hasUniqueLabels: false,
rows: [],
},
queryResponse: {
...initialState.queryResponse,
state: LoadingState.Streaming,
},
};
reducerTester<ExploreItemState>()
.givenReducer(timeReducer, initialState)
.whenActionIsDispatched(changeRefreshInterval({ exploreId: ExploreId.left, refreshInterval: 'LIVE' }))
.thenStateShouldEqual(expectedState);
});
it("should result in 'done' state, when live-tailing is stopped", () => {
const initialState = makeExplorePaneState();
const expectedState = {
...initialState,
refreshInterval: '',
logsResult: {
hasUniqueLabels: false,
rows: [],
},
queryResponse: {
...initialState.queryResponse,
state: LoadingState.Done,
},
};
reducerTester<ExploreItemState>()
.givenReducer(timeReducer, initialState)
.whenActionIsDispatched(changeRefreshInterval({ exploreId: ExploreId.left, refreshInterval: '' }))
.thenStateShouldEqual(expectedState);
});
});
describe('changing range', () => {
describe('when changeRangeAction is dispatched', () => {
it('then it should set correct state', () => {

View File

@@ -165,7 +165,6 @@ export const timeReducer = (state: ExploreItemState, action: AnyAction): Explore
},
isLive: live,
isPaused: live ? false : state.isPaused,
loading: live,
logsResult,
};
}

View File

@@ -53,7 +53,6 @@ export const makeExplorePaneState = (): ExploreItemState => ({
to: null,
} as any,
scanning: false,
loading: false,
queryKeys: [],
isLive: false,
isPaused: false,

View File

@@ -135,7 +135,6 @@ export interface ExploreItemState {
*/
scanRange?: RawTimeRange;
loading: boolean;
/**
* Table model that combines all query table results into a single table.
*/