Skip to content

Commit

Permalink
[Rollups] Add noOpSearchStrategy to handle rollup searches when rollu…
Browse files Browse the repository at this point in the history
…ps are disabled (#24798)

* Surface error within Dashboard panel.
  • Loading branch information
cjcenizal authored Nov 1, 2018
1 parent 50d3596 commit 2e590a8
Show file tree
Hide file tree
Showing 4 changed files with 77 additions and 3 deletions.
44 changes: 44 additions & 0 deletions src/ui/public/courier/search_strategy/no_op_search_strategy.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
/*
* Licensed to Elasticsearch B.V. under one or more contributor
* license agreements. See the NOTICE file distributed with
* this work for additional information regarding copyright
* ownership. Elasticsearch B.V. licenses this file to you under
* the Apache License, Version 2.0 (the "License"); you may
* not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/

import { SearchError } from './search_error';

export const noOpSearchStrategy = {
id: 'noOp',

search: async () => {
const searchError = new SearchError({
status: '418', // "I'm a teapot" error
title: 'No search strategy registered',
message: `Couldn't find a search strategy for the search request`,
type: 'NO_OP_SEARCH_STRATEGY',
path: '',
});

return {
searching: Promise.reject(searchError),
abort: () => {},
failedSearchRequests: [],
};
},

isViable: () => {
return true;
},
};
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@
* under the License.
*/

import { noOpSearchStrategy } from './no_op_search_strategy';

const searchStrategies = [];

export const addSearchStrategy = searchStrategy => {
Expand Down Expand Up @@ -48,7 +50,14 @@ const getSearchStrategyForSearchRequest = searchRequest => {

// Otherwise try to match it to a strategy.
const indexPattern = searchRequest.source.getField('index');
return getSearchStrategyByViability(indexPattern);
const viableSearchStrategy = getSearchStrategyByViability(indexPattern);

if (viableSearchStrategy) {
return viableSearchStrategy;
}

// This search strategy automatically rejects with an error.
return noOpSearchStrategy;
};


Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@ import {
addSearchStrategy,
} from './search_strategy_registry';

import { noOpSearchStrategy } from './no_op_search_strategy';

describe('SearchStrategyRegistry', () => {
describe('assignSearchRequestsToSearchStrategies', () => {
test('associates search requests with valid search strategies', () => {
Expand Down Expand Up @@ -64,7 +66,6 @@ describe('SearchStrategyRegistry', () => {
};

const searchRequests = [ searchRequest0, searchRequest1, searchRequest2, searchRequest3];

const searchStrategiesWithSearchRequests = assignSearchRequestsToSearchStrategies(searchRequests);

expect(searchStrategiesWithSearchRequests).toEqual([{
Expand All @@ -75,5 +76,20 @@ describe('SearchStrategyRegistry', () => {
searchRequests: [ searchRequest1, searchRequest2 ],
}]);
});

test(`associates search requests with noOpSearchStrategy when a viable one can't be found`, () => {
const searchRequest0 = {
id: 0,
source: { getField: () => {}, getPreferredSearchStrategyId: () => {} },
};

const searchRequests = [ searchRequest0 ];
const searchStrategiesWithSearchRequests = assignSearchRequestsToSearchStrategies(searchRequests);

expect(searchStrategiesWithSearchRequests).toEqual([{
searchStrategy: noOpSearchStrategy,
searchRequests: [ searchRequest0 ],
}]);
});
});
});
7 changes: 6 additions & 1 deletion src/ui/public/visualize/loader/visualize_data_loader.ts
Original file line number Diff line number Diff line change
Expand Up @@ -96,17 +96,22 @@ export class VisualizeDataLoader {
return this.visData;
} catch (error) {
params.searchSource.cancelQueued();

this.vis.requestError = error;
this.vis.showRequestError = error.type && error.type === 'UNSUPPORTED_QUERY';
this.vis.showRequestError =
error.type && ['NO_OP_SEARCH_STRATEGY', 'UNSUPPORTED_QUERY'].includes(error.type);

// tslint:disable-next-line
console.error(error);

if (isTermSizeZeroError(error)) {
return toastNotifications.addDanger(
`Your visualization ('${this.vis.title}') has an error: it has a term ` +
`aggregation with a size of 0. Please set it to a number greater than 0 to resolve ` +
`the error.`
);
}

toastNotifications.addDanger({
title: 'Error in visualization',
text: error.message,
Expand Down

0 comments on commit 2e590a8

Please sign in to comment.