-
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
Move buildEsQuery to a package #23345
Changes from 2 commits
e27ba53
1308202
0480b9c
72fbe52
7c93a4b
391060f
fb63b64
dafa660
a96841a
09a69ed
844037d
96f133f
a2fb932
8f2480c
63d230d
3884518
cb0aea2
0a0a6d6
09e4331
b783ea9
36b4259
f124678
d8bfa36
414c311
a13a749
4fbac25
887c97a
103ee2b
d1d0970
59f978f
e36d7ab
f0149b1
99a8bc0
245f72c
991d46f
04f5e37
9fd38fe
7db5961
df364f0
408ddb4
ffa8657
ee4ddc2
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 |
---|---|---|
|
@@ -18,6 +18,9 @@ | |
*/ | ||
|
||
export { SearchSourceProvider } from './search_source'; | ||
export { migrateFilter } from './migrate_filter'; | ||
export { decorateQuery } from './decorate_query'; | ||
export { buildQueryFromFilters, luceneStringToDsl } from './build_query'; | ||
export { | ||
migrateFilter, | ||
decorateQuery, | ||
buildQueryFromFilters, | ||
luceneStringToDsl | ||
} from '../../../../utils/es_query'; | ||
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 we even need to export these from here now are can existing users of this module just import the functions from the new directory? 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. We could, it just seemed like fewer changes to do it this way rather than go through each of the imports and change them. I'm open to changing this if you think it makes sense. 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. Maybe I missed some, but it looked to me like the imports had already been changed. Intellij's refactoring tools should handle it. I'd just rip the band aid off cleanly so folks in the future aren't confused about where they should be importing from. 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. Made the change. Wasn't even bad at all |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -82,7 +82,8 @@ import { searchRequestQueue } from '../search_request_queue'; | |
import { FetchSoonProvider } from '../fetch'; | ||
import { FieldWildcardProvider } from '../../field_wildcard'; | ||
import { getHighlightRequest } from '../../../../core_plugins/kibana/common/highlight'; | ||
import { BuildESQueryProvider } from './build_query'; | ||
import { BuildESQueryProvider } from '../../../../utils/es_query'; | ||
import { KbnError } from '../../errors'; | ||
|
||
const FIELDS = [ | ||
'type', | ||
|
@@ -601,7 +602,11 @@ export function SearchSourceProvider(Promise, Private, config) { | |
_.set(flatData.body, '_source.includes', remainingFields); | ||
} | ||
|
||
flatData.body.query = buildESQuery(flatData.index, flatData.query, flatData.filters); | ||
try { | ||
flatData.body.query = buildESQuery(flatData.index, flatData.query, flatData.filters); | ||
} catch (e) { | ||
throw new KbnError(e.message, KbnError); | ||
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. Ok now that I see this, I'm assuming that's why you removed the wildcard error type? |
||
} | ||
|
||
if (flatData.highlightAll != null) { | ||
if (flatData.highlightAll && flatData.body.query) { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -17,6 +17,4 @@ | |
* under the License. | ||
*/ | ||
|
||
export * from './ast'; | ||
export * from './filter_migration'; | ||
export * from './node_types'; | ||
export * from '../../../utils/kuery'; | ||
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. Can we just delete this module now? 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. Same comment as above, we could, but we'd have to update all of the references, which is fine by me if you think it's best. |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,4 +1,5 @@ | ||
{ | ||
"id": "logstash-*", | ||
"title": "logstash-*", | ||
"fields": [ | ||
{ | ||
|
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.
Out of interest: why are we ignoring those two files for linting? :)
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.
They're generated by PEG