algolia / algoliasearch-django

Seamless integration of Algolia into your Django project.
https://www.algolia.com
MIT License
173 stars 65 forks source link

'disable_auto_indexing()' connects receiver even if AUTO_INDEXING is False #286

Open kasai-mitsuru opened 4 years ago

kasai-mitsuru commented 4 years ago

Description

disable_auto_indexing() in decorators.py makes connection even if AUTO_INDEXING = False .

In AUTO_INDEXING = False environment, once disable_auto_indexing() is called, AlgoliaEngine will try to index when model saving.

    def __exit__(self, exc_type, exc_value, traceback):
        for model in self.models:
            post_save.connect(
                algolia_engine._AlgoliaEngine__post_save_receiver,
                sender=model
            )
            pre_delete.connect(
                algolia_engine._AlgoliaEngine__pre_delete_receiver,
                sender=model
            )

I think it needs condition if AUTO_INDEXING is True, like bellow.

    def __exit__(self, exc_type, exc_value, traceback):
        if some_auto_indexing  # this
            for model in self.models:
                post_save.connect(
                    algolia_engine._AlgoliaEngine__post_save_receiver,
                    sender=model
                )
                pre_delete.connect(
                    algolia_engine._AlgoliaEngine__pre_delete_receiver,
                    sender=model
                )

Steps To Reproduce

settings.py

ALGOLIA = {"APPLICATION_ID": "foo", "API_KEY": "bar", "AUTO_INDEXING": False}

index.py

@register(SomeModel)
class SomeModelIndex():
    ...

sample.py

from algoliasearch_django.decorators import disable_auto_indexing
with disable_auto_indexing():
    pass
SomeModel().save()  # -> Error will be Occured

a part of error message

algoliasearch.helpers.AlgoliaException: Unreachable hosts: {'foo.algolia.net': "ConnectionError: HTTPSConnectionPool(host='foo.algolia.net', port=443): Max retries exceeded with url: /1/indexes/SomeModel/236 (Caused by NewConnectionError('<urllib3.connection.VerifiedHTTPSConnection object at 0x7f126590d550>: Failed to establish a new connection: [Errno -2] Name or service not known',))", 'foo-1.algolianet.com': "ConnectionError: HTTPSConnectionPool(host='foo-1.algolianet.com', port=443): Max retries exceeded with url: /1/indexes/SomeModel/236 (Caused by NewConnectionError('<urllib3.connection.VerifiedHTTPSConnection object at 0x7f1265941da0>: Failed to establish a new connection: [Errno -2] Name or service not known',))", 'foo-3.algolianet.com': "ConnectionError: HTTPSConnectionPool(host='foo-3.algolianet.com', port=443): Max retries exceeded with url: /1/indexes/SomeModel/236 (Caused by NewConnectionError('<urllib3.connection.VerifiedHTTPSConnection object at 0x7f126590dc88>: Failed to establish a new connection: [Errno -2] Name or service not known',))", 'foo-2.algolianet.com': "ConnectionError: HTTPSConnectionPool(host='foo-2.algolianet.com', port=443): Max retries exceeded with url: /1/indexes/SomeModel/236 (Caused by NewConnectionError('<urllib3.connection.VerifiedHTTPSConnection object at 0x7f12656d8208>: Failed to establish a new connection: [Errno -2] Name or service not known',))"}
tkrugg commented 3 years ago

Hi @kasai-mitsuru that's a great point! I see another problem with this API: it's disabling indexing over all the models whilst giving the illusion it's only affecting the scope it's encapsulating. This whole API should be re-engineered into something more flexible that can enable/disable indexing on specific registrations.