Conversation
| using=self._get_client(), | ||
| index=self._get_index(), | ||
| ) | ||
| ).extra(track_total_hits=True).query() |
There was a problem hiding this comment.
Think the ES7 upgrade stuff needs to be split into its own branch, but will make a note about this now.
This should be set in an independent step to keep the coupling between the base query object and the steps that build up that query object loose (we shouldn't force the children queries to always report exact hits in other words). You can follow same pattern of adding an .extra() parameter as is done for source here:
Then add that step (called something like self.add_total_hits()) to the query building instructions here:
Also I don't think we want to call .query() here unless the DSL has somehow changed. You can see it is called explicitly in other places e.g.:
There was a problem hiding this comment.
I will get the changes requested in this branch: https://github.com/ENCODE-DCC/snovault/tree/SNO-172-ES7-upgrade with just the upgrade related commit. The DSL has changed in a way that breaks the test assertions. Without calling the .query(), top level field, "query": {} part of the overall query object isn't returned. Let me know which you prefer changed, test assertions or the parent class here.
There was a problem hiding this comment.
Empty Search no longer defaults to match_all query and instead leaves the query key empty. This is backwards incompatible when using suggest.
Kk. My guess is that we want to update the tests that expect this default to be returned, while making sure the tests that explicitly add a query still do so.
2 participants
No description provided.