feat: Improved Meilisearch Index upgrade workflow #37627
+16
−28
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| api.rebuild_index(self.stdout.write, incremental=True) | ||
| else: | ||
| api.rebuild_index(self.stdout.write) | ||
| api.rebuild_index.delay() |
Contributor
There was a problem hiding this comment.
How will the administrator know when it's done?
| ) | ||
| def test_reindex_meilisearch_collection_error(self, mock_meilisearch) -> None: | ||
|
|
||
| mock_logger = Mock() |
Contributor
There was a problem hiding this comment.
You can delete this mock and the related assertion below if we're not using it anymore. Same in the next test.
| # Changing them on a populated index will "re-index all documents in the index", which can take some time | ||
| # and use more RAM. Instead, we configure an empty index then populate it one course/library at a time. | ||
| if not incremental: | ||
| _configure_index(index_name) |
Contributor
There was a problem hiding this comment.
We still need to call _configure_index I think. We need something like this:
if not _is_index_configured(STUDIO_INDEX_NAME):
if should_reset: # Was --reset passed? Delete the whole index and start over
reset_index()
elif (# of documents in index < 2_000) or in_place: # Did user specify --in-place?
_configure_index() # Change the index in place. May be slow if it's populated with lots of data!
else:
raise Error("The index needs to be migrated to the newer format. Pass --reset to delete all index data and rebuild the index from scratch, or --in-place to update the index configuration in place (may take a long time and slow down search results in the meantime).")Although if we test this with a large dataset and find that the "in place" is always faster than rebuilding the whole index, we should just do that automatically, and this can be significantly simplified.
| # and use more RAM. Instead, we configure an empty index then populate it one course/library at a time. | ||
| if not incremental: | ||
| _configure_index(index_name) | ||
| with nullcontext(STUDIO_INDEX_NAME) as index_name: |
Contributor
There was a problem hiding this comment.
You can just remove this whole line/context.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
This PR is for issue 36868.
Features:
rebuild_studio.Testing instructions
Enable
MEILISEARCH_ENABLEDlocally.Add
openedx.core.djangoapps.meilisearchin installed apps.