-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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
Add SearchStrategyRegistry and defaultSearchStrategy to support existing search behavior, and integrate it with CallClient. #20497
Changes from 27 commits
2bd4962
1c1b99a
a0386ea
ebd3544
27ba85c
8a7e05f
0a8442b
9777a60
31925d5
b74224c
9d02ff2
d48f65b
bcce546
5c7666c
6b97760
3964b3a
596a8c9
dcddc1c
00a8fe5
ae033f2
5bd4b01
d867a3f
714990c
683a2f3
5ad6a55
464b568
6d4fc57
4aba1fa
4c51a51
1c4e668
ff73efb
e6379d8
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -25,8 +25,14 @@ import { | |
DiscoverNoResults, | ||
} from './no_results'; | ||
|
||
import { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. references to rollup message can be removed with implementation of |
||
DiscoverUnsupportedRollup, | ||
} from './unsupported_rollup'; | ||
|
||
import './timechart'; | ||
|
||
const app = uiModules.get('apps/discover', ['react']); | ||
|
||
app.directive('discoverNoResults', reactDirective => reactDirective(DiscoverNoResults)); | ||
|
||
app.directive('discoverUnsupportedRollup', reactDirective => reactDirective(DiscoverUnsupportedRollup)); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,43 @@ | ||
/* | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. then, this component can be moved somewhere inside x-pack/rollups directory as part of rollup PR |
||
* 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 React, { Fragment } from 'react'; | ||
|
||
import { | ||
EuiCallOut, | ||
EuiFlexGroup, | ||
EuiFlexItem, | ||
EuiSpacer, | ||
} from '@elastic/eui'; | ||
|
||
export const DiscoverUnsupportedRollup = () => ( | ||
<Fragment> | ||
<EuiSpacer size="xl" /> | ||
|
||
<EuiFlexGroup justifyContent="center"> | ||
<EuiFlexItem grow={false} className="discoverNoResults"> | ||
<EuiCallOut | ||
title="OSS Kibana doesn't support rollup index patterns. Please use X-Pack to search this index pattern." | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do you think users will understand "OSS Kibana"? I wonder if we should rephrase this to instead say something like: "Index Patterns based on rollup indices require the rollup plugin from X-Pack which is either not installed or disabled." There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I like this a lot. @gchaps thoughts? |
||
color="danger" | ||
iconType="alert" | ||
/> | ||
</EuiFlexItem> | ||
</EuiFlexGroup> | ||
</Fragment> | ||
); |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -60,6 +60,10 @@ <h1 tabindex="0" id="kui_local_breadcrumb" class="kuiLocalBreadcrumb"> | |
|
||
<div class="discover-wrapper col-md-10"> | ||
<div class="discover-content"> | ||
<discover-unsupported-rollup | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. and this will use generic |
||
ng-if="isUnsupportedRollup" | ||
></discover-unsupported-rollup> | ||
|
||
<discover-no-results | ||
ng-show="resultState === 'none'" | ||
top-nav-toggle="kbnTopNav.toggle" | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
when rollup reference is abstracted away, maybe this can be something like:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I redact my previous review after chatting with CJ and realizing that the unsupported rollup messages were are due to giving user hints when they're using OSS. X-Pack code isn't reachable in that scenario.
The comprise to support this type of error in OSS, while avoiding cross stream OSS/X-Pack code and rollup references, is to check whether the index pattern is the default type, instead of checking for rollup type.