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

Handle foreign key automatically #13

Closed
maxiloc opened this issue Aug 10, 2015 · 20 comments
Closed

Handle foreign key automatically #13

maxiloc opened this issue Aug 10, 2015 · 20 comments

Comments

@maxiloc
Copy link
Contributor

maxiloc commented Aug 10, 2015

Might be the same as #2

@dethi
Copy link
Contributor

dethi commented Aug 10, 2015

Yep. The problem is to detect that it is a foreign key. I'm still investigating the Django's core to find a way to do it.

@dethi
Copy link
Contributor

dethi commented Aug 10, 2015

Is it a request from a customer?

@maxiloc
Copy link
Contributor Author

maxiloc commented Aug 10, 2015

yep

@arnaudlimbourg
Copy link
Contributor

Starting in Django 1.8 you have the Meta API. My understanding is you could use MyModel._meta.get_field(name) to find more info about the field. This api has always existed but was only formalised iin 1.8.

In previous Django versions you have to use the old api but the documentation explains it better than I would.

@cabello
Copy link

cabello commented Oct 5, 2015

Yes, I recommend using the Meta API for Django 1.8, get_fields will return all model fields, and is_relation property will tell you if it's a foreign key.

@jdotjdot
Copy link

I think this is the same as this issue and #2--I'm not even able to get through the installation instructions in the README because the first time I run python manage.py algolia_reindex, I get a KeyError that looks like it's due to trying to call a RelatedManager (from a ForeignKey) simply because it's callable:

Traceback (most recent call last):
  File "manage.py", line 10, in <module>
    execute_from_command_line(sys.argv)
  File "/Users/JJ/.virtualenvs/TCJ/lib/python2.7/site-packages/django/core/management/__init__.py", line 385, in execute_from_command_line
    utility.execute()
  File "/Users/JJ/.virtualenvs/TCJ/lib/python2.7/site-packages/django/core/management/__init__.py", line 377, in execute
    self.fetch_command(subcommand).run_from_argv(self.argv)
  File "/Users/JJ/.virtualenvs/TCJ/lib/python2.7/site-packages/django/core/management/base.py", line 288, in run_from_argv
    self.execute(*args, **options.__dict__)
  File "/Users/JJ/.virtualenvs/TCJ/lib/python2.7/site-packages/django/core/management/base.py", line 338, in execute
    output = self.handle(*args, **options)
  File "/Users/JJ/.virtualenvs/TCJ/lib/python2.7/site-packages/django/contrib/algoliasearch/management/commands/algolia_reindex.py", line 22, in handle
    batch_size=options.get('batchsize', None))
  File "/Users/JJ/.virtualenvs/TCJ/lib/python2.7/site-packages/django/contrib/algoliasearch/models.py", line 224, in reindex_all
    batch.append(self._build_object(instance))
  File "/Users/JJ/.virtualenvs/TCJ/lib/python2.7/site-packages/django/contrib/algoliasearch/models.py", line 149, in _build_object
    attr = attr()
  File "/Users/JJ/.virtualenvs/TCJ/lib/python2.7/site-packages/django/db/models/fields/related.py", line 654, in __call__
    manager = getattr(self.model, kwargs.pop('manager'))
KeyError: u'manager'

The attr in question that is being called is the reverse RelatedManager for a related model with a foreign key on my model.

If I could declare attributes in the AlgoliaIndex class that would let me override and specify how I want to handle certain fields, similar to how you can in Django Rest Framework serializers, that would probably do the trick here. I may do this by subclassing AlgoliaIndex and rewriting my own _build_object() method, or by just using DRF serializers and bypassing algoliasearch-django entirely (as much as I'd like to use it).

@dethi
Copy link
Contributor

dethi commented Oct 30, 2015

Hey @jdotjdot,
Your issue certainly comes from this line. I think you can fix it by creating a "model index" with the fields defined to what you really need to index (take a look at the end of the Quick Start).

There is a lot of bugs fix and new features in the develop branch that need documentation. I will try to do it as soon as I have some free time! I'm sorry for the long waiting time 😞

@thetylerhayes
Copy link

Just wanted to say I was able to get this working thanks to @dethi's comment ^^ but I'll also throw my hat in the ring that it was frustrating when I was setting this up and thinking I'd get up and running quickly and having a roadblock pretty much immediately in the form of an ambiguous exception I then had to Google. :) Thanks for making a great product overall though!

EDIT: to clarify, I was able to get it working by specifying model fields manually, and specifically excluding ForeignKey fields. E.g., I had to do:

class CommentIndex(AlgoliaIndex):
    fields = ('raw_message')

instead of:

class CommentIndex(AlgoliaIndex):
    fields = ('user', 'raw_message')

where user is a ForeignKey.

@calitidexvii
Copy link

calitidexvii commented Apr 22, 2016

Is there a workaround to include fields from the ForeignKey model within the index? For example, using @thetylerhayes model:

class CommentIndex(AlgoliaIndex):
    fields = ('user.first_name', 'raw_message')

@dethi
Copy link
Contributor

dethi commented Apr 22, 2016

Hi @calitidexvii
Not for now sorry. The issue is that during object update, these fields are not necessary available and will need to be fetched from the database.

@calitidexvii
Copy link

Thanks @dethi
Based on your research, do you think it's something that could be added if you had the help? For example:

class CommentIndex(AlgoliaIndex):
    fields = ('raw_message')
    fk_fields = ('user.first_name')

Where fk_fields would perform the DB lookup and append to the object?

@calitidexvii
Copy link

For anyone else needing a workaround, I was able to use callables within the model for the index.

class Comment(models.Model):
    user = models.ForeignKey(User)
    raw_message = models.TextField()

    def user_filter(self):
        return [self.user.username]

Then the index:

class CommentIndex(AlgoliaIndex):
    fields = ('raw_message')
    tags = 'user_filter'

This isn't perfect and may not work in all situations, but it seems to be sufficient when you need to manage user access with secured API keys.

@abendig
Copy link

abendig commented Sep 21, 2016

Would love to hear if there is any news as to when this might be available -- or how best to help make this a reality.

This is an increasingly important use case for us. One example: Indexing User profile data, where some information may be in the User table and other information is in a UserProfile table.

@calitidexvii
Copy link

@abendig Due to the issue @dethi mentioned, I don't think there are any plans to extend the integration to handle relations.

Do you have a working solution? If not, you could try my workaround above - using callables to get the necessary data. Due to increasingly complex relations, I ultimately ended up dumping the Django integration and writing custom methods using only the Python integration. If you need some direction, let me know.

If you need/want the management functions of the Django integration, it may be worth forking and extending AlgoliaIndex to make the database calls for relations (caveat emptor).

@calitidexvii
Copy link

@abendig I just noticed, this was added to the 2.0.0 milestone in June 2016, but I don't see a timeline for it.

@dethi
Copy link
Contributor

dethi commented Sep 21, 2016

Hi,
Sorry. I don't work anymore at Algolia so it's only a side project for me.
The milestone 2.0.0 has a due date, end of October, and I will fulfil it in time.
I hope it won't be too late for you!

@abendig
Copy link

abendig commented Sep 21, 2016

Thanks a lot for the update. I do need to make this functionality work one way or another in the next couple weeks. How far along is this particular feature? Is it possible/useful to get involved in some way to drive the development of this forward?

@dethi
Copy link
Contributor

dethi commented Sep 25, 2016

I would appreciate if you could provide me a django app that fakes your use case.

@PLNech
Copy link
Member

PLNech commented Apr 24, 2017

Closed as stale without an answer from OP.

@PLNech
Copy link
Member

PLNech commented Nov 23, 2017

For future readers, the readme now contains a section on indexing relations.

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

No branches or pull requests

9 participants