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

[Fix #111] Adding API for filtering object while indexing #301

Merged
merged 3 commits into from Apr 17, 2021

Conversation

safwanrahman
Copy link
Collaborator

@safwanrahman safwanrahman commented Oct 31, 2020

This fixes #111. We need to add documentation for this. But I am not sure where to add it.
overwriting the should_index_object and returning a boolean value should determine which object should be index.

For example, if we want to index Car only with Red color, we can write following

class Car(Document):
    def should_index_object(self, obj):
        if obj.color == "RED":
            return True
        else:
            return False

@safwanrahman
Copy link
Collaborator Author

@barseghyanartur @alexgarel Any thoughts about the code and where should we document it? any suggestion about the API or anything?

@barseghyanartur
Copy link
Contributor

@safwanrahman:

What about in here: es_index? Seems quite logical to me.

@safwanrahman
Copy link
Collaborator Author

@barseghyanartur I was also thinking same. Lets see what @alexgarel thinks about this, as he is good at documentations.

@alexgarel
Copy link
Collaborator

What about in here: es_index? Seems quite logical to me.

I think this is a good idea ! It's not that at this point documentation is in a ideal state, but better put this information somewhere and move it latter, if needed.

Also put a docstring on should_index_object explaining the feature. (a lot of us look directly in the code + it would autodocument when we setup sphinx to do so)

@willbarton
Copy link

We've discovered the bug described in #111 and were looking at potential fixes and work-arounds. Is this PR just waiting on documentation?

@safwanrahman
Copy link
Collaborator Author

@willbarton Yes. it is waiting for the documentation. It will be great help if anyone can write something up!

@barseghyanartur
Copy link
Contributor

@safwanrahman:

Shall I just do this? And it could be improved later on (if needed).

@safwanrahman:

What about in here: es_index? Seems quite logical to me.

@safwanrahman
Copy link
Collaborator Author

@barseghyanartur Sure! 🚀

@safwanrahman safwanrahman merged commit ce5f697 into django-es:master Apr 17, 2021
@safwanrahman
Copy link
Collaborator Author

Seems like we need to merge this rathar than waiting for documentation! Added docstring though! 😅

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Updating index do not respect get_queryset method
4 participants