Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support for Elasticsearch 5.x #1461

Closed
wants to merge 10 commits into from

Conversation

ElSaico
Copy link
Contributor

@ElSaico ElSaico commented Dec 13, 2016

Based on #1460 - will be properly rebased once it's accepted.

So far only two features seem to be failing, according to the test suite:

  • Faceted search (ES seems to throw an error about fielddata)
  • Autocomplete

Any help to solve any of those would be appreciated.

@seocam
Copy link

seocam commented Dec 13, 2016

This PR addresses #1383

@Alkalit
Copy link

Alkalit commented Dec 13, 2016

Hi! I also work on elasticsearch 5 support for haystack. My project lies in this repo.
I think it would be better if are new backends (and also are some old) will be placed in separate repos.
The reason for this is that testing for all these backends complicates the testing process. And it imposes an additional burden
at its core developers, since in addition to the main project code must still follow trends for each search engine.

What do you say if I offer you to work together at this project? Or at least borrow the idea.

@acdha
Copy link
Contributor

acdha commented Jan 2, 2017

I just merged #1460 so it might be good to see how small this request can be

@ElSaico ElSaico changed the title [WIP] Support for Elasticsearch 5.x Support for Elasticsearch 5.x Jan 4, 2017
@seocam
Copy link

seocam commented Jan 4, 2017

@ElSaico travis has whitelisted elasticsearch 5.x repo. We can update/simplify travis.yml ;)

@ElSaico
Copy link
Contributor Author

ElSaico commented Jan 4, 2017

@Alkalit just managed to fix the last of the issues, but thank you for your offer!

@acdha the major source of shared code is build_search_kwargs, which is true on 1.x/2.x as well. I've refactored all three to address this, which can be seen here: https://github.com/CraveFood/django-haystack-elasticsearch
Perhaps a PR to do this on the current backends would be a good preceding step?

@acdha
Copy link
Contributor

acdha commented Jan 4, 2017

@ElSaico That seems reasonable – I've been meaning to take a look at that in general and seeing how much we could refactor into shared utilities or methods on BaseSearchBackend so things like input parameter validation & type conversion would be consistent across all of the backends.

Things like https://github.com/django-haystack/django-haystack/blob/master/haystack/backends/elasticsearch_backend.py#L309 or

if models and len(models):
model_choices = sorted(get_model_ct(model) for model in models)
elif limit_to_registered_models:
# Using narrow queries, limit the results to only models handled
# with the current routers.
model_choices = self.build_models_list()
else:
model_choices = []
come to mind.

@bqumsiyeh
Copy link

bqumsiyeh commented Jan 26, 2017

What is the timeframe for merging this into a stable release?

@seocam
Copy link

seocam commented Jan 26, 2017

We have been using ES5 with haystack in our production for about 20 days now. For our needs it seems reasonably stable.

@acdha
Copy link
Contributor

acdha commented Jan 26, 2017

@seocam thanks - I can do a release soon but since I don't use it I'm not entirely comfortable saying it's been well tested

@seocam
Copy link

seocam commented Jan 26, 2017

@acdha I totally understand! Even I would be a lot more comfortable with more people using it from the branch before having a new version out.

@acdha
Copy link
Contributor

acdha commented Jan 26, 2017

Maybe I'll merge it and publish a -pre release to make it easier for people to test

@mintusah25
Copy link

mintusah25 commented Feb 15, 2017

@acdha Any update on pre release for ES 5.X??.

@rrmerugu
Copy link

@acdha any update on pre release for ES 5.x !

@SalahAdDin
Copy link

SalahAdDin commented Apr 3, 2017

@acdha Please.

@adamchainz
Copy link
Contributor

@SalahAdDin typo? I don't have anything to do with django-haystack

@Ameriks
Copy link

Ameriks commented Apr 17, 2017

ES 5.x support branch doesn't work for me.

File "/src/django-haystack/haystack/urls.py", line 7, in <module>
from haystack.views import SearchView

File "/src/django-haystack/haystack/views.py", line 16, in <module>
class SearchView(object):

File "/src/django-haystack/haystack/views.py", line 20, in SearchView
results = EmptySearchQuerySet()

File "/src/django-haystack/haystack/query.py", line 29, in __init__
self._determine_backend()

File "/src/django-haystack/haystack/query.py", line 62, in _determine_backend
self.query = connections[backend_alias].get_query()

File "/src/django-haystack/haystack/utils/loading.py", line 108, in __getitem__
self.thread_local.connections[key] = load_backend(self.connections_info[key]['ENGINE'])(using=key)

TypeError: __init__() missing 1 required positional argument: 'connection_alias'

Sorry, false issue. I included wrong class in settings.py. Included Elasticsearch5SearchBackend instead of Elasticsearch5SearchEngine.

Well done!

@alanjds
Copy link

alanjds commented Apr 18, 2017

Is there anything holding it to be merged yet?

@acdha
Copy link
Contributor

acdha commented Apr 18, 2017

@alanjds More test results from people using ES would be helpful

@alanjds
Copy link

alanjds commented Apr 19, 2017

@acdha I see. Looks like I would have the 1st cobaia project using this... But the good news is that nobody said "It have a problem. Be never merged. Beware and run to the hills!".

Thanks.

@acdha
Copy link
Contributor

acdha commented Apr 19, 2017

@alanjds the time taken to merge it isn't high but I don't really have that much time for an in-depth review. If you have some positive test results that'd save me the time there.

@millerf
Copy link

millerf commented Apr 28, 2017

Could it be possible to merge it?

@acdha
Copy link
Contributor

acdha commented Apr 28, 2017

@millerf If you have time to work on this, test results are appreciated

@millerf
Copy link

millerf commented Apr 28, 2017

I have very little experience with all that ES, so I am a bit perplexed...
From what I see, it is missing the sort= option on facetting.
Also, the repo ask to remove faceted=True options on SearchIndex. Is that because of ES 5.x? I suppose haystack docs should be updated for that as well, no?

acdha referenced this pull request May 18, 2017
Merge pull request #1369 from janwin/fix-empty-list-convert
@erikdao
Copy link

erikdao commented May 22, 2018

When will this be official?

@lokeshpahal
Copy link

lokeshpahal commented Jun 8, 2018

ES 5.x got issue while executing rebuild_index command

Traceback (most recent call last):
File "manage.py", line 15, in
execute_from_command_line(sys.argv)
File "/var/www/MSDev/env/lib/python3.6/site-packages/django/core/management/init.py", line 371, in execute_from_command_line
utility.execute()
File "/var/www/MSDev/env/lib/python3.6/site-packages/django/core/management/init.py", line 365, in execute
self.fetch_command(subcommand).run_from_argv(self.argv)
File "/var/www/MSDev/env/lib/python3.6/site-packages/django/core/management/base.py", line 288, in run_from_argv
self.execute(*args, **cmd_options)
File "/var/www/MSDev/env/lib/python3.6/site-packages/django/core/management/base.py", line 335, in execute
output = self.handle(*args, **options)
File "/var/www/MSDev/env/lib/python3.6/site-packages/haystack/management/commands/rebuild_index.py", line 36, in handle
call_command('clear_index', **options)
File "/var/www/MSDev/env/lib/python3.6/site-packages/django/core/management/init.py", line 133, in call_command
', '.join(sorted(valid_options)),
TypeError: Unknown option(s) for clear_index command: batchsize, workers. Valid options are: commit, help, interactive, no_color, nocommit, noinput, pythonpath, settings, skip_checks, stderr, stdout, traceback, using, verbosity, version.

@erikdao
Copy link

erikdao commented Jun 8, 2018 via email

@chocobn69
Copy link
Contributor

All tests are passing on my project. I will use it on production and tell you results in a few days.

Copy link
Contributor

@chocobn69 chocobn69 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Working on production system since 72h, no issues.

@acdha
Copy link
Contributor

acdha commented Jun 13, 2018

Thanks for testing - I’ll merge it later today

@angvp
Copy link

angvp commented Jun 17, 2018

This might need a rebase before merge, we are waiting for you @acdha :-)

@acdha
Copy link
Contributor

acdha commented Jun 17, 2018 via email

@acdha
Copy link
Contributor

acdha commented Jun 18, 2018

I merged this in my working branch (see acdha@e366f49) but we still need to fix the GIS code to work with Django 2:

https://travis-ci.org/acdha/django-haystack/jobs/393751311#L2309-L2319

@acdha
Copy link
Contributor

acdha commented Jun 18, 2018

Rebased and merged with 1fda20e to fix the get_coords calls

@acdha acdha closed this Jun 18, 2018
@UmanShahzad
Copy link

Can this be pushed to pypi?

@intellisense
Copy link

Hi, When this will be released? Thanks!

@randomlock
Copy link

I get this exception when searching data
elasticsearch.exceptions.RequestError: TransportError(400, 'parsing_exception', 'no [query] registered for [filtered]')

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet