Feature/4879 #5036
Feature/4879 #5036
Conversation
| /** | ||
| * Test building and serialising a template search request. | ||
| * */ | ||
| public class TemplateQueryBuilderTest { |
There was a problem hiding this comment.
this should extends ElasticsearchTestCase
|
I think this looks pretty awesome. I left some comments but in general we are very close here. Lets chat about the dependency and how we handle that... |
|
Thanks a lot for your comments - fixing now. Dependency: Sure. |
|
thank you! |
| "{\"match_{{template}}\": {}}\"", vars); | ||
| SearchResponse sr = client().prepareSearch().setQuery(builder) | ||
| .execute().actionGet(); | ||
| assertEquals("Template query didn't return correct number of hits.", 2, |
There was a problem hiding this comment.
you could also use ElasticsearchAssertions.assertHitCount here
|
I like the functionality a lot (just what I need it in one of my playground projects :-) Two minor things that came to my mind while reading the PR:
|
| [[query-dsl-template-query]] | ||
| === Template Query | ||
|
|
||
| A query that accepts a query template and a map of key/value pairs to fill in |
There was a problem hiding this comment.
I think we need to add a coming[1.1.0] here?
|
Few notes:
|
| ((Mustache) template).execute(writer, vars); | ||
| try { | ||
| writer.flush(); | ||
| writer.close(); |
There was a problem hiding this comment.
minor nitpick - can we move the writer.close() into a finally?
|
this looks great to me we only need to speak about the including of Guava in the package... lets talk soon |
| - do: | ||
| indices.refresh: {} | ||
|
|
||
| - do: |
There was a problem hiding this comment.
cool stuff maybe we can add a test that use another query than match_all and two documents to add a bit more variance?
|
I did another review, ran the tests and did a release build to make sure it all works and starts up etc. This looks awesome. I left one more comment (minor) but once that is fixed I am +1 to push |
|
Above commit changes the yaml test from using match_all to using a term query. |
|
awesome! Isabel please squash the commits into one and feel free to push to |
PR for #4879