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

Projects
None yet
@ElSaico
Copy link
Contributor

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

This comment has been minimized.

Copy link

commented Dec 13, 2016

This PR addresses #1383

@Alkalit

This comment has been minimized.

Copy link

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

This comment has been minimized.

Copy link
Contributor

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

This comment has been minimized.

Copy link

commented Jan 4, 2017

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

@ElSaico

This comment has been minimized.

Copy link
Contributor Author

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?

@ElSaico ElSaico force-pushed the CraveFood:es5-support branch from 3762a97 to d761d12 Jan 4, 2017

@acdha

This comment has been minimized.

Copy link
Contributor

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

This comment has been minimized.

Copy link

commented Jan 26, 2017

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

@seocam

This comment has been minimized.

Copy link

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

This comment has been minimized.

Copy link
Contributor

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

This comment has been minimized.

Copy link

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

This comment has been minimized.

Copy link
Contributor

commented Jan 26, 2017

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

@mintusah25

This comment has been minimized.

Copy link

commented Feb 15, 2017

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

@rrmerugu

This comment has been minimized.

Copy link

commented Mar 20, 2017

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

@SalahAdDin

This comment has been minimized.

Copy link

commented Apr 3, 2017

@acdha Please.

@adamchainz

This comment has been minimized.

Copy link
Contributor

commented Apr 3, 2017

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

@Ameriks

This comment has been minimized.

Copy link

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

This comment has been minimized.

Copy link

commented Apr 18, 2017

Is there anything holding it to be merged yet?

@acdha

This comment has been minimized.

Copy link
Contributor

commented Apr 18, 2017

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

@alanjds

This comment has been minimized.

Copy link

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

This comment has been minimized.

Copy link
Contributor

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

This comment has been minimized.

Copy link

commented Apr 28, 2017

Could it be possible to merge it?

@acdha

This comment has been minimized.

Copy link
Contributor

commented Apr 28, 2017

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

@millerf

This comment has been minimized.

Copy link

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

Better handling of empty lists in field preparation
Merge pull request #1369 from janwin/fix-empty-list-convert

@LuisAlejandro LuisAlejandro referenced this pull request Oct 15, 2017

Closed

is not searching #529

@erikdao

This comment has been minimized.

Copy link

commented May 22, 2018

When will this be official?

@lokeshpahal

This comment has been minimized.

Copy link

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

This comment has been minimized.

Copy link

commented Jun 8, 2018

@chocobn69

This comment has been minimized.

Copy link
Contributor

commented Jun 11, 2018

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

@chocobn69
Copy link
Contributor

left a comment

Working on production system since 72h, no issues.

@acdha

This comment has been minimized.

Copy link
Contributor

commented Jun 13, 2018

Thanks for testing - I’ll merge it later today

@angvp

This comment has been minimized.

Copy link

commented Jun 17, 2018

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

@acdha

This comment has been minimized.

Copy link
Contributor

commented Jun 17, 2018

@acdha

This comment has been minimized.

Copy link
Contributor

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

This comment has been minimized.

Copy link
Contributor

commented Jun 18, 2018

Rebased and merged with 1fda20e to fix the get_coords calls

@acdha acdha closed this Jun 18, 2018

@UmanShahzad

This comment has been minimized.

Copy link

commented Sep 27, 2018

Can this be pushed to pypi?

@intellisense

This comment has been minimized.

Copy link

commented Jan 25, 2019

Hi, When this will be released? Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.