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

Add admin methods for reindexing versions from project and version admin. #5343

Merged
merged 7 commits into from Feb 27, 2019

Conversation

@dojutsu-user
Copy link
Member

@dojutsu-user dojutsu-user commented Feb 23, 2019

Related issue - #4654

  • On Project admin, a command that reindexes all active versions (project queryset and all attached active versions)
  • On Version admin, a command that reindexes by version queryset instead
  • Drop management command, these aren't good interfaces for site admins and are mostly hidden
  • The management command seems to just do all versions active or not, we should probably test for active versions instead.
  • There should also be an admin function to wipe the index for a version and for a project
@dojutsu-user
Copy link
Member Author

@dojutsu-user dojutsu-user commented Feb 23, 2019

@ericholscher

You should be able to call https://github.com/rtfd/readthedocs.org/blob/master/readthedocs/search/signals.py#L44

(comment #1 )
Calling index_project_save() will not do anything because ELASTICSEARCH_DSL_AUTO_REFRESH is set to False and that's why I have made a helper function _indexing_helper() for this purpose.

Same reason for not calling index_html_file() directly.

@dojutsu-user
Copy link
Member Author

@dojutsu-user dojutsu-user commented Feb 23, 2019

I apologise that this PR took too much time from my side.

@ericholscher ericholscher self-requested a review Feb 25, 2019
@safwanrahman safwanrahman self-requested a review Feb 26, 2019
@ericholscher
Copy link
Member

@ericholscher ericholscher commented Feb 26, 2019

@dojutsu-user did you test this locally? I'm seeing it reindexing all projects. It looks like this happens when there is a version with no html_objs, it continues to pass it on to ES which indexes everything.

Refs #5357

else:
for version in active_versions.iterator():
html_objs = HTMLFile.objects.filter(project=project, version=version)
_indexing_helper(html_objs, wipe=False)
Copy link
Member

@ericholscher ericholscher Feb 26, 2019

This is reindexing a number of times, it should build a list of all the files and then pass it once.

Copy link
Member Author

@dojutsu-user dojutsu-user Feb 26, 2019

Thank you.
I have made and pushed the changes.

If ``wipe`` is set to False, html_objs are deleted from the ES index,
else, html_objs are indexed.
"""
from readthedocs.search.tasks import index_objects_to_es, delete_objects_in_es
Copy link
Member

@ericholscher ericholscher Feb 26, 2019

This should check if html_objs exists -- it might not.

Copy link
Member Author

@dojutsu-user dojutsu-user Feb 26, 2019

I have added the checks to prevent this.

@dojutsu-user
Copy link
Member Author

@dojutsu-user dojutsu-user commented Feb 26, 2019

@ericholscher

did you test this locally

I did tested this locally.

It looks like this happens when there is a version with no html_objs, it continues to pass it on to ES which indexes everything.

I have added multiple if to prevent this from happening.

Copy link
Member

@ericholscher ericholscher left a comment

Looks good. Thanks!

@ericholscher ericholscher merged commit 6f4b39d into readthedocs:master Feb 27, 2019
1 check passed
@dojutsu-user dojutsu-user deleted the admin-reindexing branch Feb 27, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants