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

Datasources: Propagate TestDataDB default selection to PanelModel #57703

Closed
wants to merge 10 commits into from
16 changes: 15 additions & 1 deletion e2e/various-suite/explore.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,20 @@ e2e.scenario({

// Both queries above should have been run and be shown in the query history
e2e.components.QueryTab.queryHistoryButton().should('be.visible').click();
e2e.components.QueryHistory.queryText().should('have.length', 2).should('contain', 'csv_metric_values');
e2e.components.QueryHistory.queryText()
.should('have.length.gte', 3)
.should('contain', 'csv_metric_values')
.should('contain', 'random_walk');
// Latest three queries should be [csv_metric_values, csv_metric_values, random_walk]
e2e.components.QueryHistory.queryText().each(($el, i) => {
if (i === 0 || i === 1) {
expect($el.text()).to.equal('csv_metric_values');
} else if (i === 2) {
expect($el.text()).to.equal('random_walk');
} else {
return false;
}
return true;
});
},
});
33 changes: 29 additions & 4 deletions public/app/features/query/components/QueryGroup.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -82,8 +82,25 @@ export class QueryGroup extends PureComponent<Props, State> {
const dsSettings = this.dataSourceSrv.getInstanceSettings(options.dataSource);
const defaultDataSource = await this.dataSourceSrv.get();
const datasource = ds.getRef();
const queries = options.queries.map((q) => (q.datasource ? q : { ...q, datasource }));
let changedQueries = false;
const queries = await Promise.all(options.queries.map(async (q) => {
if (!q.datasource) {
q = { ...q, datasource };
changedQueries = true;
}
if (Object.keys(q).length === 2) {
// If query has only refId and datasource, try to set the default query for its datasource
q = {...this.newQuery(await this.dataSourceSrv.get(q.datasource), dsSettings), ...q};
changedQueries = true;
}
return q;
}));
this.setState({ queries, dataSource: ds, dsSettings, defaultDataSource });
if (changedQueries) {
this.onChange({
queries
});
}
Comment on lines +99 to +103
Copy link
Member

Choose a reason for hiding this comment

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

This is basically the same as calling onChange onMount in the query editor, you could maybe refactor out to a function that can also be used from runRequest? Then we can detect and perform this logic before issuing requests as well. And then we don't need to call this.onChange (which should be preserved for use-initiated change)

Copy link
Contributor

Choose a reason for hiding this comment

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

Do we really need to call onChange? If not I think this can be simplified a bit

  const queries = options.queries.map((q) => {
        if (!q.datasource) {
          q = { ...q, datasource };
        }
        if (Object.keys(q).length === 2) {
          q = { ...this.newQuery(ds, dsSettings), ...q };
        }
        return q;
      });
      this.setState({ queries, dataSource: ds, dsSettings, defaultDataSource });

Think we may also need to set default query in [updateQueries](https://github.com/grafana/grafana/blob/mmandrus/fix-testdatadb-default-selection/public/app/features/query/state/updateQueries.ts#L30-L36) too. Otherwise, this won't work when changing the data source.

} catch (error) {
console.log('failed to load data source', error);
}
Expand All @@ -107,6 +124,9 @@ export class QueryGroup extends PureComponent<Props, State> {

// We need to pass in newSettings.uid as well here as that can be a variable expression and we want to store that in the query model not the current ds variable value
const queries = await updateQueries(nextDS, newSettings.uid, this.state.queries, currentDS);
if (queries.length === 0) {
queries.push(this.newQuery(nextDS, newSettings) as DataQuery);
}

const dataSource = await this.dataSourceSrv.get(newSettings.name);
this.onChange({
Expand All @@ -132,13 +152,18 @@ export class QueryGroup extends PureComponent<Props, State> {
this.onScrollBottom();
};

newQuery(): Partial<DataQuery> {
const { dsSettings, defaultDataSource } = this.state;
newQuery(newDs?: DataSourceApi, newSettings?: DataSourceInstanceSettings): Partial<DataQuery> {
const { defaultDataSource } = this.state;
const dsSettings = newSettings || this.state.dsSettings;

const ds = !dsSettings?.meta.mixed ? dsSettings : defaultDataSource;

let defaultQuery = (newDs || this.state.dataSource)?.getDefaultQuery?.(CoreApp.PanelEditor);
if (!defaultQuery) {
defaultQuery = { refId: 'A' };
}
return {
...this.state.dataSource?.getDefaultQuery?.(CoreApp.PanelEditor),
...defaultQuery,
datasource: { uid: ds?.uid, type: ds?.type },
};
}
Expand Down
4 changes: 2 additions & 2 deletions public/app/features/query/state/updateQueries.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,12 +27,12 @@ export async function updateQueries(
}
// Otherwise clear queries
else {
return [{ refId: 'A', datasource }];
return [];
}
}

if (nextQueries.length === 0) {
return [{ refId: 'A', datasource }];
return [];
}

// Set data source on all queries except expression queries
Expand Down
17 changes: 17 additions & 0 deletions public/app/plugins/datasource/testdata/datasource.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import { delay } from 'rxjs/operators';
import {
AnnotationEvent,
ArrayDataFrame,
CoreApp,
DataFrame,
DataQueryRequest,
DataQueryResponse,
Expand All @@ -18,6 +19,7 @@ import {
import { DataSourceWithBackend, getBackendSrv, getGrafanaLiveSrv, getTemplateSrv, TemplateSrv } from '@grafana/runtime';
import { getSearchFilterScopedVar } from 'app/features/variables/utils';

import { defaultQuery } from './constants';
import { queryMetricTree } from './metricTree';
import { generateRandomNodes, savedNodesResponse } from './nodeGraphUtils';
import { runStream } from './runStreams';
Expand Down Expand Up @@ -258,6 +260,21 @@ export class TestDataDataSource extends DataSourceWithBackend<TestDataQuery> {

return null;
}

getDefaultQuery(app: CoreApp): TestDataQuery {
switch (app) {
// Ignore the app input for now, always use the same default query
case CoreApp.CloudAlerting:
case CoreApp.UnifiedAlerting:
case CoreApp.Dashboard:
case CoreApp.Explore:
case CoreApp.PanelEditor:
case CoreApp.PanelViewer:
case CoreApp.Unknown:
default:
return defaultQuery;
}
}
Comment on lines +264 to +277
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
getDefaultQuery(app: CoreApp): TestDataQuery {
switch (app) {
// Ignore the app input for now, always use the same default query
case CoreApp.CloudAlerting:
case CoreApp.UnifiedAlerting:
case CoreApp.Dashboard:
case CoreApp.Explore:
case CoreApp.PanelEditor:
case CoreApp.PanelViewer:
case CoreApp.Unknown:
default:
return defaultQuery;
}
}
getDefaultQuery(app: CoreApp): TestDataQuery {
return defaultQuery;
}

}

function runGrafanaAPI(target: TestDataQuery, req: DataQueryRequest<TestDataQuery>): Observable<DataQueryResponse> {
Expand Down