search

mozilla / elasticutils

[deprecated] A friendly chainable ElasticSearch interface for python
http://elasticutils.rtfd.org
BSD 3-Clause "New" or "Revised" License
243 stars 76 forks source link

Ensure results cache is used even if there were no results #212

Closed nearlyfreeapps closed 10 years ago

nearlyfreeapps commented 10 years ago

On Marketplace, we determined that we were making two requests to Elasticsearch whenever we had a search that returned no results. From some debugging I believe this is because of the way that self._results_cache is being evaluated here: https://github.com/mozilla/elasticutils/blob/master/elasticutils/__init__.py#L1450. In cases with no results, the __len__ method is being being evaluated to 0 which makes this statement falsey (and triggers another request).

This will simply explicitly check if the self._results_cache is not None.

willkg commented 10 years ago

I'll look at this for 0.9 later this week.

I'm kind of puzzled, though. I thought we had fixed this already, but maybe it was a related thing and not this particular thing. I'll be enlightened when I scrutinize further.

robhudson commented 10 years ago

I can confirm that this does fix the bug we're looking at.

The count does a boolean check on self._results_cache, which in the case I'm looking at is a elasticutils.DictSearchResults object. The boolean check triggers a __len__ as mentioned. The length comes back zero which results in a False. So it does the query with a slice resulting in a 2nd ES hit.

Applying this patch fixes it and returns the count from the _result_cache.

willkg commented 10 years ago

Looks good to me. Thank you!