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

Make _build_query() accept dicts as sort values #189

Closed diox closed 10 years ago

diox commented 10 years ago

Hi,

This pull request allows developers to use dicts as sort values in order_by(). This is useful to do nested object filtering, which I'd like to use that in Firefox Marketplace to fix https://bugzilla.mozilla.org/show_bug.cgi?id=951626

willkg commented 10 years ago

This looks fine to me. It just needs some documentation in .order_by() I think.

diox commented 10 years ago

I wanted to add something in the docs but I'm unsure what to change: https://github.com/mozilla/elasticutils/blob/master/docs/searching.rst#ordering-order_by is rather generic and https://github.com/mozilla/elasticutils/blob/master/docs/api.rst just has automethod:: elasticutils.S.order_by. Thoughts ?

willkg commented 10 years ago

The automethod:: elasticutils.S.order_by thing is a Sphinx directive to pull in the docstring of the .order_by() method. So to document this, you'd need to add stuff here:

https://github.com/mozilla/elasticutils/blob/master/elasticutils/__init__.py#L605

diox commented 10 years ago

Ah, right. Added docs & rebased.

willkg commented 10 years ago

Looks good to me! Thank you!

willkg commented 10 years ago

Oh, one thing--this is against master tip and is after the pyelasticsearch -> elasticsearch-py change. The problem there being that I doubt you want to update to elasticsearch-py to pick this up for AMO.

I can cherry-pick this to the end of the branch-0.8 branch if that helps.

diox commented 10 years ago

Oh, yes, good catch, I had not noticed the update to elasticsearch-py. Cherry-picking that to 0.8 would help, yeah. And well, actually... a 0.8.2 release with that change would be great if that's not too much to ask. Nothing urgent though, we are not going to push this to production till 2014 I think :)

willkg commented 10 years ago

I created issue #190 for a 0.8.2 release. I'll try to get to it this week or next week or sometime soon.

diox commented 10 years ago

Awesome, thanks, I'll watch that issue.