ChristopherRabotin / bungiesearch

UNMAINTAINED CODE -- Elasticsearch-dsl-py django wrapper with mapping generator
BSD 3-Clause "New" or "Revised" License
67 stars 20 forks source link

BungiesearchManager.__getattr__ tries to instantiate a Bungiesearch instance when not given settings #160

Closed gracebarrington closed 7 years ago

gracebarrington commented 7 years ago

Hi all, I've noticed in our project that when Bungiesearch is "set up" but "not enabled", that is there are models using the BungiesearchManager and Bungisearch is in the INSTALLED_APPS, but in settings.py BUNGIESEARCH is an empty dict, this causes a problem with search alias lookups. We define the BUNGIESEARCH settings object as an empty dictionary unless the relevant env variables are set, at which point we fill it out, so environments without those variables set (such as new dev environments or, in our case, some of our unit tests) will encounter this problem.

The problem will manifest when trying to access an attribute on the model manager that doesn't exist, in our case it was a Django REST Framework serialiser calling getattr(rel_mgr, 'use_for_related_fields', False). Because BungiesearchManager.__getattr__ is only called when a lookup fails, we got around it by changing __getattr__ to check if settings.BUNGIESEARCH evaluated to True, and if not, raising an AttributeError like one that would normally be encountered, like so:

class SearchManager(BungiesearchManager):
    def __getattr__(self, alias):
        if settings.BUNGIESEARCH:
            return super(SearchManager, self).__getattr__(alias)
        else:
            raise AttributeError("'{0}' object has no attribute '{1}'".format(
                type(self).__name__, alias))

I'm happy to convert this to a PR if you guys are happy with that solution?

ChristopherRabotin commented 7 years ago

I see. Yes, that sounds like a valid issue, and I like the solution you propose. I'll be happy to review that PR.

On Fri, Jan 6, 2017, 10:50 Guy Barrington notifications@github.com wrote:

Hi all, I've noticed in our project that when Bungiesearch is "set up" but "not enabled", that is there are models using the BungiesearchManager and Bungisearch is in the INSTALLED_APPS, but in settings.py BUNGIESEARCH is an empty dict, this causes a problem with search alias lookups. We define the BUNGIESEARCH settings object as an empty dictionary unless the relevant env variables are set, at which point we fill it out, so environments without those variables set (such as new dev environments or, in our case, some of our unit tests) will encounter this problem.

The problem will manifest when trying to access an attribute on the model manager that doesn't exist, in our case it was a Django REST Framework serialiser calling getattr(rel_mgr, 'use_for_related_fields', False). Because BungiesearchManager.getattr is only called when a lookup fails, we got around it by changing getattr to check if settings.BUNGIESEARCH evaluated to True, and if not, raising an AttributeError like one that would normally be encountered, like so:

class SearchManager(BungiesearchManager): def getattr(self, alias): if settings.BUNGIESEARCH: return super(SearchManager, self).getattr(alias) else: raise AttributeError("'{0}' object has no attribute '{1}'".format( type(self).name, alias))

I'm happy to convert this to a PR if you guys are happy with that solution?

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/ChristopherRabotin/bungiesearch/issues/160, or mute the thread https://github.com/notifications/unsubscribe-auth/AEma6MY1XSKHdnx5FTbOP6av1hJ8N2Cgks5rPn7TgaJpZM4Lc8Zk .