Skip to content

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

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 7 commits into from
Feb 27, 2019
Merged

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

merged 7 commits into from
Feb 27, 2019

Conversation

dojutsu-user
Copy link
Member

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 commented Feb 23, 2019 •
edited
Loading

@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 dojutsu-user mentioned this pull request Feb 23, 2019
Closed
5 tasks
@dojutsu-user
Copy link
Member Author

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

@ericholscher ericholscher self-requested a review February 25, 2019 14:13
@safwanrahman safwanrahman self-requested a review February 26, 2019 17:18
@ericholscher
Copy link
Member

ericholscher commented Feb 26, 2019 •
edited
Loading

@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

ericholscher
ericholscher reviewed Feb 26, 2019
View reviewed changes
readthedocs/projects/admin.py Outdated
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 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you.
I have made and pushed the changes.

readthedocs/search/utils.py
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

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have added the checks to prevent this.

@dojutsu-user
Copy link
Member Author

@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.

ericholscher
ericholscher approved these changes Feb 27, 2019
View reviewed changes
Copy link
Member

@ericholscher ericholscher left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. Thanks!

@ericholscher ericholscher merged commit 6f4b39d into readthedocs:master Feb 27, 2019
@dojutsu-user dojutsu-user deleted the admin-reindexing branch February 27, 2019 14:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ericholscher ericholscher ericholscher approved these changes

@safwanrahman safwanrahman Awaiting requested review from safwanrahman

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@dojutsu-user @ericholscher