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

Suggestions support #185

Closed koterpillar closed 10 years ago

koterpillar commented 10 years ago

As promised in #172, suggestion support.

koterpillar commented 10 years ago

Sorry, fixed the test.

willkg commented 10 years ago

I get this error when running the tests against Elasticsearch 0.20.5:

ERROR: elasticutils.tests.test_query:SuggestionTest.test_suggestions
  vim +1313 elasticutils/tests/test_query.py  # test_suggestions
    suggestions = s.suggestions()
  vim +1503 elasticutils/__init__.py  # suggestions
    return self._do_search().response.get('suggest', {})
  vim +1287 elasticutils/__init__.py  # _do_search
    response = self.raw()
  vim +1361 elasticutils/__init__.py  # raw
    doc_type=self.get_doctypes())
  vim +70   /home/willkg/.virtualenvs/eu/lib/python2.7/site-packages/elasticsearch/client/utils.py  # _wrapped
    return func(*args, params=params, **kwargs)
  vim +388  /home/willkg/.virtualenvs/eu/lib/python2.7/site-packages/elasticsearch/client/__init__.py  # search
    params=params, body=body)
  vim +223  /home/willkg/.virtualenvs/eu/lib/python2.7/site-packages/elasticsearch/transport.py  # perform_request
    status, raw_data = connection.perform_request(method, url, params, body, ignore=ignore)
  vim +53   /home/willkg/.virtualenvs/eu/lib/python2.7/site-packages/elasticsearch/connection/http_urllib3.py  # perform_request
    self._raise_error(response.status, raw_data)
  vim +82   /home/willkg/.virtualenvs/eu/lib/python2.7/site-packages/elasticsearch/connection/base.py  # _raise_error
    raise HTTP_EXCEPTIONS.get(status_code, TransportError)(status_code, error_message, additional_info)
TransportError: TransportError(500, u'SearchPhaseExecutionException[Failed to execute phase [query], total failure; shardFailures {[tLsITzokTZ-xqTpzuUXBtQ][elasticutilstest][2]: SearchParseException[[elasticutilstest][2]: query[name:mary],from[-1],size[-1]: Parse Failure [Failed to parse source [{"query": {"text": {"name": "mary"}}, "suggest": {"mysuggest": {"text": "mary", "term": {"field": "_all"}}}}]]]; nested: SearchParseException[[elasticutilstest][2]: query[name:mary],from[-1],size[-1]: Parse Failure [No parser for element [suggest]]]; }{[tLsITzokTZ-xqTpzuUXBtQ][elasticutilstest][0]: SearchParseException[[elasticutilstest][0]: query[name:mary],from[-1],size[-1]: Parse Failure [Failed to parse source [{"query": {"text": {"name": "mary"}}, "suggest": {"mysuggest": {"text": "mary", "term": {"field": "_all"}}}}]]]; nested: SearchParseException[[elasticutilstest][0]: query[name:mary],from[-1],size[-1]: Parse Failure [No parser for element [suggest]]]; }{[tLsITzokTZ-xqTpzuUXBtQ][elasticutilstest][1]: SearchParseException[[elasticutilstest][1]: query[name:mary],from[-1],size[-1]: Parse Failure [Failed to parse source [{"query": {"text": {"name": "mary"}}, "suggest": {"mysuggest": {"text": "mary", "term": {"field": "_all"}}}}]]]; nested: SearchParseException[[elasticutilstest][1]: query[name:mary],from[-1],size[-1]: Parse Failure [No parser for element [suggest]]]; }]')
koterpillar commented 10 years ago

Suggestion API is available since Elasticsearch version 0.90. What is Elasticutils' policy about this - should I specify the minimum ES version in the method documentation and check in tests, throw an UnsupportedESVersion or GetOutOfTheCave exception, or ignore?

willkg commented 10 years ago

There really isn't a policy.

We added support for "match" queries and noted in the documentation that it worked in ES 0.19.9 and later. That's the only related thing I can think of.

I think two things should probably happen.

  1. the documentation should specify the minimum ES version this works with
  2. the tests should skip this test if the ES it's connecting to isn't recent enough

There's currently no infrastructure for item 2. I know I wrote some code a while back that told me what version ES thought it was. I think you'll have to do something similar. Might be good to have as a lazy property in ESTestCase or something.

koterpillar commented 10 years ago

Both done.

willkg commented 10 years ago

This looks good to me. Thank you!

willkg commented 10 years ago

Couldn't automatically merge, so I cherry-picked by hand: