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

Adding "all results" feature support for slicing. #167

Closed mindflayer closed 11 years ago

mindflayer commented 11 years ago

Are you interested in this feature? If so, I'll add some tests.

willkg commented 11 years ago

Hi! I can't really tell what I'm looking at. What is this? What problem does this solve? What's the use case?

mindflayer commented 11 years ago

How do you get all the elements skipping -let's say- the first five?

In Python: resultset[5:]

willkg commented 11 years ago

Is this for paging? What's the use case for getting all the elements and skipping the first five?

mindflayer commented 11 years ago

Please note: at the moment the above syntax is perfectly valid for elasticutils, but is working in a non-standard way.

If I write: resultset[5:] is giving me a slice of ten elements (elasticsearch default is 10 elements) starting from the sixth.

Yes, paging is -of course- a use case.

mindflayer commented 11 years ago

Of course, I haven't finished yet, but if you agree I'll work on it. I hate working for nothing. ;)

willkg commented 11 years ago

It sounds like you want to fix the behavior to be more correct in regards to standard Python slicing behavior. That makes sense to me.

However, I don't see a valid use case for why anyone would find themselves in this situation, certainly not with paging.

Regardless, the way you're fixing this causes ElasticUtils to do an additional ES request to get the count. I don't want to do that. If you can figure out a way that doesn't involve hitting ES for the count, then I'm game for looking at that.

Until then, I think we should make add a note about this unintended behavior to the documentation.

willkg commented 11 years ago

In the future, to avoid the working-for-nothing situation, it's better to open up an issue that has a detailed account of what the problem is.

mindflayer commented 11 years ago

Yes, you right. Sometimes what is clear to me it's not to others, sorry for that.

mindflayer commented 11 years ago

@willkg: Am I wrong if I say the second query is made using self._results_cache content?

willkg commented 11 years ago

Enable logging and see what happens.

mindflayer commented 11 years ago

I have an idea, working on it in my spare time.

willkg commented 11 years ago

I'm going to nix this PR for now. It'd be good to write up an issue with the use case so that it's getting tracked. Open a new PR when you have something ready to be looked at.

Thank you!