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

@grafana/runtime: Add cancellation of queries to DataSourceWithBackend #22818

Merged
merged 1 commit into from
Mar 17, 2020

Conversation

marefr
Copy link
Member

@marefr marefr commented Mar 16, 2020

What this PR does / why we need it:
Uses the cancellation of queries from #22643 and closing that PR for now.

@marefr marefr added this to the 6.7 milestone Mar 16, 2020
@marefr marefr requested a review from ryantxu March 16, 2020 22:24
.then((rsp: any) => {
return this.toDataQueryResponse(rsp);
return this.toDataQueryResponse(rsp?.data);
Copy link
Member

Choose a reason for hiding this comment

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

Why is the .data needed?

Copy link
Member Author

Choose a reason for hiding this comment

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

Because the datasourceRequest returns a different response object than when using post. It returns

export interface DataSourceResponse<T> {
data: T;
readonly status: number;
readonly statusText: string;
readonly ok: boolean;
readonly headers: Headers;
readonly redirected: boolean;
readonly type: ResponseType;
readonly url: string;
readonly request: any;
}

Copy link
Member

Choose a reason for hiding this comment

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

👍

const orgId = config.bootData.user.orgId;
const queries = targets.map(q => {
if (q.datasource === ExpressionDatasourceID) {
expressionCount++;
Copy link
Member

Choose a reason for hiding this comment

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

Is this hack not needed anymore? If not where are the backend changes that go with it?

Copy link
Member Author

Choose a reason for hiding this comment

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

Doesn't look like it. The expressionCount was provided in body on root level but the grafana backend didn't receive that at all.

@marefr marefr requested a review from ryantxu March 17, 2020 16:11
Copy link
Member

@ryantxu ryantxu left a comment

Choose a reason for hiding this comment

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

looks good -- thanks

@marefr marefr merged commit eb96a8f into master Mar 17, 2020
@marefr marefr deleted the ds_query_cancel branch March 17, 2020 23:12
bergquist added a commit that referenced this pull request Mar 18, 2020
* master: (113 commits)
  AlertNotifications: Translate notifications IDs to UIDs in Rule builder (#19882)
  CircleCI: Don't build Storybook on PR (#22865)
  Graphite: Rollup Indicator (#22738)
  Plugins: Return jsondetails as an json object instead of raw json on datasource healthchecks. (#22859)
  Backend plugins: Exclude plugin metrics in Grafana's metrics endpoint (#22857)
  Graphite: Fixed issue with query editor and next select metric now showing after selecting metric node  (#22856)
  Webpack: Fix webpack for enterprise (#22863)
  Metrics: Storybook documented components  (#22854)
  Search: Improve tags layout , #22804 (#22830)
  Stackdriver: Fix GCE auth bug when creating new data source (#22836)
  @grafana/runtime: Add cancellation of queries to DataSourceWithBackend (#22818)
  Rich history: Test coverage (#22852)
  Chore: Support Volta in package.json (#22849)
  CircleCI: Skip enterprise builds for forked PRs (#22851)
  Toolkit: docker image for plugin CI process (#22790)
  Revert "Explore: Add test coverage for Rich history (#22722)" (#22850)
  Datasource config was not mapped for datasource healthcheck (#22848)
  upgrades plugin sdk to 0.30.0 (#22846)
  Explore: Add test coverage for Rich history (#22722)
  Rich History: UX adjustments and fixes (#22729)
  ...
aknuds1 pushed a commit that referenced this pull request Mar 19, 2020
@marefr marefr mentioned this pull request Mar 19, 2020
aknuds1 pushed a commit that referenced this pull request Mar 19, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants