Switch aggregations from push to pull #19839
Conversation
nik9000
added
>enhancement
review
:Search/Search
|
woops, compile error with a testing plugin. will fix. |
|
@rjernst would you like to look at this one? It is the last "plugin API" change for search. |
| /** | ||
| * Package private with the intent that it is used for testing. | ||
| */ | ||
| interface TestPlugin extends SearchPlugin { |
There was a problem hiding this comment.
I don't think we need test code in production. Instead, can you add this method to Node, and have MockNode override it? Eventually, we can change it to createSearchService, once we can construct IndicesService without guice (really close..we can probably do that in a follow up, all the pieces are attainable).
|
Looks ok to me. I'm still not a fan of the generics, but they are in line with what you have for the rest of the search plugin methods. The one comment I left is my only ask. |
|
@rjernst - I moved the testing hook into MockNode. It turns out that MockNode was actually broken so I had to rework it some. Would you like to take a look? |
|
|
||
| public MockNode(Settings settings, Collection<Class<? extends Plugin>> classpathPlugins) { | ||
| super(InternalSettingsPreparer.prepareEnvironment(settings, null), classpathPlugins); | ||
| this.plugins = classpathPlugins; | ||
| this.mockBigArrays = classpathPlugins.contains(NodeMocksPlugin.class); // if this plugin is present we mock bigarrays :) |
There was a problem hiding this comment.
This didn't work because we use the value of mockBigArrays in the super call above. But we hadn't yet set mockBigArrays.
|
@rjernst I pushed an update that did the thing you wanted. |
|
LGTM |
Adds `getAggregations` to `SearchPlugin` which can be used to register aggregations. Fixup MockNode which wasn't createing MockBigArrays.
nik9000
removed
the
:Search/Search
Adds
getAggregationsandgetPipelineAggregationstoSearchPluginwhich can be used to registeraggregations. Makes overriding
onModule(SearchModule)in aPlugina compile error.Also removes many
ParseFields which are now implied by the registration mechanism that we expose to plugins.