liberation / django-elasticsearch

Simple wrapper around elasticsearch-py to index/search a django Model.
MIT License
211 stars 73 forks source link

Fix errors with django 1.9.X; #48

Closed onegreyonewhite closed 8 years ago

onegreyonewhite commented 8 years ago

This is working with Elasticsearch 2.3.1 and Django 1.9.7. Fix for #31

lauxley commented 8 years ago

I get for every version of django:

======================================================================
ERROR: test_auto_delete (django_elasticsearch.tests.test_indexable.EsAutoIndexTestCase)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "../django_elasticsearch/tests/test_indexable.py", line 214, in setUp
    verbosity=2)
  File "/home/robin/Projects/django-elasticsearch/test_project/.tox/py27-django18/local/lib/python2.7/site-packages/django/dispatch/dispatcher.py", line 189, in send
    response = receiver(signal=self, sender=sender, **named)
  File "../django_elasticsearch/models.py", line 76, in es_syncdb_callback
    for model in sender.get_models():
AttributeError: 'NoneType' object has no attribute 'get_models'

======================================================================
ERROR: test_auto_save (django_elasticsearch.tests.test_indexable.EsAutoIndexTestCase)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "../django_elasticsearch/tests/test_indexable.py", line 214, in setUp
    verbosity=2)
  File "/home/robin/Projects/django-elasticsearch/test_project/.tox/py27-django18/local/lib/python2.7/site-packages/django/dispatch/dispatcher.py", line 189, in send
    response = receiver(signal=self, sender=sender, **named)
  File "../django_elasticsearch/models.py", line 76, in es_syncdb_callback
    for model in sender.get_models():
AttributeError: 'NoneType' object has no attribute 'get_models'

----------------------------------------------------------------------

Which is consistent with what i experienced and why i didn't added the fix in #31 in the first place, if you look at models.py:

if getattr(settings, 'ELASTICSEARCH_AUTO_INDEX', False):
    # Note: can't specify the sender class because EsIndexable is Abstract,
    # see: https://code.djangoproject.com/ticket/9318
    post_save.connect(es_save_callback)
    post_delete.connect(es_delete_callback)
    post_migrate.connect(es_syncdb_callback)

The comment make it clear why sender is None here, so i don't know how it works for you :)

lauxley commented 8 years ago

Also, if you call do_update() in the test i think you are not actually testing the auto indexing, but i think it is fine because we can assume Django is working as expected !

onegreyonewhite commented 8 years ago

I tested this in real data and models. This test was from your branch. Tomorrow I will see the code and send you the changes.

onegreyonewhite commented 8 years ago

@lauxley, I fix some my mistakes. Tests for EsAutoIndexTestCase in 1.6,1.7,1.8,1.9 are passed. For 1.4 I have message:

Traceback (most recent call last):
  File "manage.py", line 13, in <module>
    execute_from_command_line(sys.argv)
  File "/home/grey/django-elasticsearch/test_project/.tox/py27-django14/local/lib/python2.7/site-packages/django/core/management/__init__.py", line 443, in execute_from_command_line
    utility.execute()
  File "/home/grey/django-elasticsearch/test_project/.tox/py27-django14/local/lib/python2.7/site-packages/django/core/management/__init__.py", line 382, in execute
    self.fetch_command(subcommand).run_from_argv(self.argv)
  File "/home/grey/django-elasticsearch/test_project/.tox/py27-django14/local/lib/python2.7/site-packages/django/core/management/commands/test.py", line 49, in run_from_argv
    super(Command, self).run_from_argv(argv)
  File "/home/grey/django-elasticsearch/test_project/.tox/py27-django14/local/lib/python2.7/site-packages/django/core/management/base.py", line 196, in run_from_argv
    self.execute(*args, **options.__dict__)
  File "/home/grey/django-elasticsearch/test_project/.tox/py27-django14/local/lib/python2.7/site-packages/django/core/management/base.py", line 232, in execute
    output = self.handle(*args, **options)
  File "/home/grey/django-elasticsearch/test_project/.tox/py27-django14/local/lib/python2.7/site-packages/django/core/management/commands/test.py", line 72, in handle
    failures = test_runner.run_tests(test_labels)
  File "/home/grey/django-elasticsearch/test_project/.tox/py27-django14/local/lib/python2.7/site-packages/django/test/simple.py", line 380, in run_tests
    suite = self.build_suite(test_labels, extra_tests)
  File "/home/grey/django-elasticsearch/test_project/.tox/py27-django14/local/lib/python2.7/site-packages/django/test/simple.py", line 261, in build_suite
    suite.addTest(build_test(label))
  File "/home/grey/django-elasticsearch/test_project/.tox/py27-django14/local/lib/python2.7/site-packages/django/test/simple.py", line 108, in build_test
    "or app.TestCase.test_method" % label)
ValueError: Test label 'django_elasticsearch.tests.test_indexable.EsAutoIndexTestCase' should be of the form app.TestCase or app.TestCase.test_method

It is important at all?

lauxley commented 8 years ago

I don't really understand what you mean by ''I tested this in real data and models.", if you have your own hooks for indexing (through django signals or something else) you shouldn't expect the given callback to do what you want. I mean that es_save_callback for example is not meant to be a general purpose signal callback and should NOT be used outside of the scope of auto/generic indexing, and in this case we can't get around the fact that abstract models don't set the sender on signals.

To be more specific, don't use these callbacks with your own signals hook, it won't work, as expected ! Either set ELASTICSEARCH_AUTO_INDEX to True, or call my_instance.es.do_index() yourself.

onegreyonewhite commented 8 years ago

Let me explain. With this pull request I try only to correct problems associated with the transition to the 1.9.x version of Django. In saying that, I tested the performance on real data, I meant that in my test project is implemented not only through the Django test, but with scripts simulating work in combat conditions.Whatever, the request solve the problem of transition to version 1.9. Unfortunately I can not lay out the source of your test project.