From a94ba7784aa55c3a7d25500333c1e65640bc1e0b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Piotr=20Jamr=C3=B3z?= Date: Tue, 20 Jun 2023 17:38:05 +0200 Subject: [PATCH] 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 --- .../components/rule-editor/VizWrapper.tsx | 3 +- public/app/features/explore/Explore.tsx | 6 +-- .../explore/ExploreQueryInspector.tsx | 6 +-- .../app/features/explore/ExploreToolbar.tsx | 15 ++---- .../features/explore/Graph/GraphContainer.tsx | 1 - .../features/explore/Logs/LogsContainer.tsx | 3 +- .../RawPrometheus/RawPrometheusContainer.tsx | 4 +- .../features/explore/Table/TableContainer.tsx | 4 +- .../features/explore/state/datasource.test.ts | 1 - .../app/features/explore/state/datasource.ts | 1 - public/app/features/explore/state/query.ts | 39 +++++++++------ .../app/features/explore/state/time.test.ts | 49 +------------------ public/app/features/explore/state/time.ts | 1 - public/app/features/explore/state/utils.ts | 1 - public/app/types/explore.ts | 1 - 15 files changed, 46 insertions(+), 89 deletions(-) diff --git a/public/app/features/alerting/unified/components/rule-editor/VizWrapper.tsx b/public/app/features/alerting/unified/components/rule-editor/VizWrapper.tsx index 22f31ccc3a0..12c76bb5b4d 100644 --- a/public/app/features/alerting/unified/components/rule-editor/VizWrapper.tsx +++ b/public/app/features/alerting/unified/components/rule-editor/VizWrapper.tsx @@ -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) => {
{isTimeSeriesData ? ( { } renderGraphPanel(width: number) { - const { graphResult, absoluteRange, timeZone, queryResponse, loading, showFlameGraph } = this.props; + const { graphResult, absoluteRange, timeZone, queryResponse, showFlameGraph } = this.props; return ( 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( diff --git a/public/app/features/explore/Graph/GraphContainer.tsx b/public/app/features/explore/Graph/GraphContainer.tsx index 2413636300d..56f0fd17b08 100644 --- a/public/app/features/explore/Graph/GraphContainer.tsx +++ b/public/app/features/explore/Graph/GraphContainer.tsx @@ -19,7 +19,6 @@ import { ExploreGraphLabel } from './ExploreGraphLabel'; import { loadGraphStyle } from './utils'; interface Props extends Pick { - loading: boolean; data: DataFrame[]; annotations?: DataFrame[]; eventBus: EventBus; diff --git a/public/app/features/explore/Logs/LogsContainer.tsx b/public/app/features/explore/Logs/LogsContainer.tsx index b39c4df0a23..5bf3791770a 100644 --- a/public/app/features/explore/Logs/LogsContainer.tsx +++ b/public/app/features/explore/Logs/LogsContainer.tsx @@ -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]; diff --git a/public/app/features/explore/RawPrometheus/RawPrometheusContainer.tsx b/public/app/features/explore/RawPrometheus/RawPrometheusContainer.tsx index fa10a7f55c7..f3a9e03b65b 100644 --- a/public/app/features/explore/RawPrometheus/RawPrometheusContainer.tsx +++ b/public/app/features/explore/RawPrometheus/RawPrometheusContainer.tsx @@ -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; diff --git a/public/app/features/explore/Table/TableContainer.tsx b/public/app/features/explore/Table/TableContainer.tsx index 34ad8d8d807..9daaacce0b2 100644 --- a/public/app/features/explore/Table/TableContainer.tsx +++ b/public/app/features/explore/Table/TableContainer.tsx @@ -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 }; } diff --git a/public/app/features/explore/state/datasource.test.ts b/public/app/features/explore/state/datasource.test.ts index 606db7151f4..1218c074d67 100644 --- a/public/app/features/explore/state/datasource.test.ts +++ b/public/app/features/explore/state/datasource.test.ts @@ -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. diff --git a/public/app/features/explore/state/datasource.ts b/public/app/features/explore/state/datasource.ts index cf2dd78b26f..18e183ae05b 100644 --- a/public/app/features/explore/state/datasource.ts +++ b/public/app/features/explore/state/datasource.ts @@ -104,7 +104,6 @@ export const datasourceReducer = (state: ExploreItemState, action: AnyAction): E logsResult: null, supplementaryQueries: loadSupplementaryQueries(), queryResponse: createEmptyQueryResponse(), - loading: false, queryKeys: [], history, }; diff --git a/public/app/features/explore/state/query.ts b/public/app/features/explore/state/query.ts index 5df6c6b37a1..5fb64eb154f 100644 --- a/public/app/features/explore/state/query.ts +++ b/public/app/features/explore/state/query.ts @@ -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, diff --git a/public/app/features/explore/state/time.test.ts b/public/app/features/explore/state/time.test.ts index d6c7175ae54..3b74ab41018 100644 --- a/public/app/features/explore/state/time.test.ts +++ b/public/app/features/explore/state/time.test.ts @@ -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() - .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() - .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', () => { diff --git a/public/app/features/explore/state/time.ts b/public/app/features/explore/state/time.ts index a9872c81500..0d7056aed17 100644 --- a/public/app/features/explore/state/time.ts +++ b/public/app/features/explore/state/time.ts @@ -165,7 +165,6 @@ export const timeReducer = (state: ExploreItemState, action: AnyAction): Explore }, isLive: live, isPaused: live ? false : state.isPaused, - loading: live, logsResult, }; } diff --git a/public/app/features/explore/state/utils.ts b/public/app/features/explore/state/utils.ts index cbc2de0282a..7d39cd26033 100644 --- a/public/app/features/explore/state/utils.ts +++ b/public/app/features/explore/state/utils.ts @@ -53,7 +53,6 @@ export const makeExplorePaneState = (): ExploreItemState => ({ to: null, } as any, scanning: false, - loading: false, queryKeys: [], isLive: false, isPaused: false, diff --git a/public/app/types/explore.ts b/public/app/types/explore.ts index b62d8f47c4f..b3d53fa7e58 100644 --- a/public/app/types/explore.ts +++ b/public/app/types/explore.ts @@ -135,7 +135,6 @@ export interface ExploreItemState { */ scanRange?: RawTimeRange; - loading: boolean; /** * Table model that combines all query table results into a single table. */