Skip to content

feat: (sceneQueryRunner) - allow manual timerange override, pass props to useQueryRunner #1098

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

Closed
wants to merge 1 commit into from
Closed
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
8 changes: 6 additions & 2 deletions packages/scenes-react/src/hooks/useQueryRunner.ts
Original file line number Diff line number Diff line change
@@ -1,17 +1,19 @@
import { useEffect } from 'react';
import { SceneDataQuery, SceneQueryRunner } from '@grafana/scenes';
import { SceneDataQuery, SceneQueryRunner, SceneTimeRangeLike } from '@grafana/scenes';
import { DataSourceRef } from '@grafana/schema';
import { isEqual } from 'lodash';
import { CacheKey } from '../caching/SceneObjectCache';
import { useSceneObject } from './useSceneObject';

export interface UseQueryOptions {
queries: SceneDataQuery[];
maxDataPoints?: number;
datasource?: DataSourceRef;
cacheKey?: CacheKey;
liveStreaming?: boolean;
maxDataPointsFromWidth?: boolean;
// Optional timeRange to use instead of sceneGraph timeRange
runQueriesMode?: 'auto' | 'manual';
timeRange?: SceneTimeRangeLike;
Comment on lines +14 to +16
Copy link
Member

Choose a reason for hiding this comment

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

  1. the comment is on the wrong prop
  2. Feels like passing a SceneTimeRangeLike is wrong here vs a plain TimeRange prop. Also would need to add diffing so when prop changes query is executed (when runQueriesMode is auto)
  3. VizPanel uses sceneGraph.getTimeRange as well to change time range, for zoom in on graphs so that will not work with this approach.

}

/**
Expand All @@ -31,6 +33,8 @@ export function useQueryRunner(options: UseQueryOptions): SceneQueryRunner {
datasource: options.datasource,
liveStreaming: options.liveStreaming,
maxDataPointsFromWidth: options.maxDataPointsFromWidth,
runQueriesMode: options.runQueriesMode,
timeRange: options.timeRange,
}),
objectConstructor: SceneQueryRunner,
cacheKey: options.cacheKey,
Expand Down
91 changes: 91 additions & 0 deletions packages/scenes/src/querying/SceneQueryRunner.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2754,6 +2754,97 @@ describe.each(['11.1.2', '11.1.1'])('SceneQueryRunner', (v) => {

expect(sentRequest?.scopes).toBeUndefined();
});

describe('when timeRange is provided as prop', () => {
it('should use provided timeRange instead of sceneGraph timeRange', async () => {
const customTimeRange = new SceneTimeRange({
from: '2023-02-01',
to: '2023-02-02',
});

const queryRunner = new SceneQueryRunner({
queries: [{ refId: 'A' }],
timeRange: customTimeRange,
});

const scene = new TestScene({
$timeRange: new SceneTimeRange({
from: '2023-03-01',
to: '2023-03-02',
}),
$data: queryRunner,
});

scene.activate();
queryRunner.activate();

await new Promise((r) => setTimeout(r, 1));

expect(sentRequest?.range).toEqual(customTimeRange.state.value);
});

it('should not subscribe to sceneGraph timeRange changes when using prop timeRange', async () => {
const customTimeRange = new SceneTimeRange({
from: '2023-01-01',
to: '2023-01-02',
});

const queryRunner = new SceneQueryRunner({
queries: [{ refId: 'A' }],
timeRange: customTimeRange,
});

const sceneTimeRange = new SceneTimeRange({
from: '2023-02-01',
to: '2023-02-02',
});

const scene = new TestScene({
$timeRange: sceneTimeRange,
$data: queryRunner,
});

scene.activate();
queryRunner.activate();

await new Promise((r) => setTimeout(r, 1));

// Change scene time range
sceneTimeRange.setState({
from: '2023-03-01',
to: '2023-03-02',
});
sceneTimeRange.onRefresh();

await new Promise((r) => setTimeout(r, 1));

// Should still use custom time range
expect(sentRequest?.range).toEqual(customTimeRange.state.value);
});

it('should fall back to sceneGraph timeRange when prop timeRange is not provided', async () => {
const sceneTimeRange = new SceneTimeRange({
from: '2023-01-01',
to: '2023-01-02',
});

const queryRunner = new SceneQueryRunner({
queries: [{ refId: 'A' }],
});

const scene = new TestScene({
$timeRange: sceneTimeRange,
$data: queryRunner,
});

scene.activate();
queryRunner.activate();

await new Promise((r) => setTimeout(r, 1));

expect(sentRequest?.range).toEqual(sceneTimeRange.state.value);
});
});
});

class CustomDataSource extends RuntimeDataSource {
Expand Down
23 changes: 17 additions & 6 deletions packages/scenes/src/querying/SceneQueryRunner.ts
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,8 @@ export interface QueryRunnerState extends SceneObjectState {
dataLayerFilter?: DataLayerFilter;
// Private runtime state
_hasFetchedData?: boolean;
// Optional timeRange to use instead of sceneGraph timeRange
timeRange?: SceneTimeRangeLike;
Copy link
Member

Choose a reason for hiding this comment

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

not sure this makes sense, all scene objects can already have local time range via $timeRange

}

export interface DataQueryExtended extends DataQuery {
Expand Down Expand Up @@ -140,9 +142,14 @@ export class SceneQueryRunner extends SceneObjectBase<QueryRunnerState> implemen
this.addActivationHandler(() => this._onActivate());
}

private getTimeRange(): SceneTimeRangeLike {
// If timeRange is provided in state, use that, otherwise fall back to sceneGraph
return this.state.timeRange ?? sceneGraph.getTimeRange(this);
}

private _onActivate() {
if (this.isQueryModeAuto()) {
const timeRange = sceneGraph.getTimeRange(this);
const timeRange = this.getTimeRange();
const scopesBridge = sceneGraph.getScopesBridge(this);

// Add subscriptions to any extra providers so that they rerun queries
Expand Down Expand Up @@ -188,7 +195,7 @@ export class SceneQueryRunner extends SceneObjectBase<QueryRunnerState> implemen
}

private _onLayersReceived(results: Iterable<SceneDataProviderResult>) {
const timeRange = sceneGraph.getTimeRange(this);
const timeRange = this.getTimeRange();
const { dataLayerFilter } = this.state;

let annotations: DataFrame[] = [];
Expand Down Expand Up @@ -312,7 +319,7 @@ export class SceneQueryRunner extends SceneObjectBase<QueryRunnerState> implemen
}

private _isDataTimeRangeStale(data: PanelData) {
const timeRange = sceneGraph.getTimeRange(this);
const timeRange = this.getTimeRange();

const stateTimeRange = timeRange.state.value;
const dataTimeRange = data.timeRange;
Expand Down Expand Up @@ -390,13 +397,17 @@ export class SceneQueryRunner extends SceneObjectBase<QueryRunnerState> implemen
this._scopesSubBridge = scopesBridge;

this._scopesSub = scopesBridge.subscribeToValue(() => {
this.runWithTimeRangeAndScopes(sceneGraph.getTimeRange(this), scopesBridge);
this.runWithTimeRangeAndScopes(this.getTimeRange(), scopesBridge);
});
}

private subscribeToTimeRangeChanges(timeRange: SceneTimeRangeLike) {
// Only subscribe to time range changes if we're not using a prop
if (this.state.timeRange) {
return;
}

if (this._timeSubRange === timeRange) {
// Nothing to do, already subscribed
return;
}

Expand All @@ -411,7 +422,7 @@ export class SceneQueryRunner extends SceneObjectBase<QueryRunnerState> implemen
}

public runQueries() {
const timeRange = sceneGraph.getTimeRange(this);
const timeRange = this.getTimeRange();
const scopesBridge = sceneGraph.getScopesBridge(this);
if (this.isQueryModeAuto()) {
this.subscribeToTimeRangeChanges(timeRange);
Expand Down
Loading