fixup! feat: Adds a way to find the difference in Meiliseach state and come up with a migration plan and configuration plan depending on the state. This introduces a mechanism it or a drift engine which drill down the Meiliseach configuration and figures out what has changed:

Author: farhaanbukhsh

openedx/core/djangoapps/content/search/api.py

@@ -245,28 +245,49 @@ def _index_is_empty(index_name: str) -> bool:
     return index.get_stats().number_of_documents == 0
 
 
-def _configure_index(index_name):
+def _apply_index_settings(
+    index_name: str,
+    wait: bool,
+    status_cb: Callable[[str], None] | None = None,
+) -> None:
     """
-    Configure the index. The following index settings are best changed on an empty index.
-    Changing them on a populated index will "re-index all documents in the index", which can take some time.
+    Apply the standard Meilisearch settings to an index.
+
+    When wait=False, settings are sent in fire-and-forget mode. This is appropriate
+    for empty temporary indexes that will immediately be populated on the same Meilisearch
+    task queue.
+
+    When wait=True, each settings task is synchronously waited on before returning.
+    This is appropriate when reconciling a live index and we need confirmation that the
+    settings have been applied before returning.
 
     Args:
-        index_name (str): The name of the index to configure
+        index_name: The name of the index to configure.
+        wait: Whether to wait for each Meilisearch settings task to complete.
+        status_cb: Optional callback for status messages when wait=True.
     """
+    if status_cb is None:
+        status_cb = log.info
+
     client = _get_meilisearch_client()
+    index = client.index(index_name)
+
+    settings_updates = (
+        ("distinct attribute", index.update_distinct_attribute, INDEX_DISTINCT_ATTRIBUTE),
+        ("filterable attributes", index.update_filterable_attributes, INDEX_FILTERABLE_ATTRIBUTES),
+        ("searchable attributes", index.update_searchable_attributes, INDEX_SEARCHABLE_ATTRIBUTES),
+        ("sortable attributes", index.update_sortable_attributes, INDEX_SORTABLE_ATTRIBUTES),
+        ("ranking rules", index.update_ranking_rules, INDEX_RANKING_RULES),
+    )
 
-    # Mark usage_key as unique (it's not the primary key for the index, but nevertheless must be unique):
-    client.index(index_name).update_distinct_attribute(INDEX_DISTINCT_ATTRIBUTE)
-    # Mark which attributes can be used for filtering/faceted search:
-    client.index(index_name).update_filterable_attributes(INDEX_FILTERABLE_ATTRIBUTES)
-    # Mark which attributes are used for keyword search, in order of importance:
-    client.index(index_name).update_searchable_attributes(INDEX_SEARCHABLE_ATTRIBUTES)
-    # Mark which attributes can be used for sorting search results:
-    client.index(index_name).update_sortable_attributes(INDEX_SORTABLE_ATTRIBUTES)
+    for label, update_method, value in settings_updates:
+        status_cb(f"Applying {label} to '{index_name}'...")
+        if wait:
+            _wait_for_meili_task(update_method(value))
+        else:
+            update_method(value)
 
-    # Update the search ranking rules to let the (optional) "sort" parameter take precedence over keyword relevance.
-    # cf https://www.meilisearch.com/docs/learn/core_concepts/relevancy
-    client.index(index_name).update_ranking_rules(INDEX_RANKING_RULES)
+    status_cb(f"All settings applied to '{index_name}'.")
 
 
 def _recurse_children(block, fn, status_cb: Callable[[str], None] | None = None) -> None:
@@ -345,7 +366,7 @@ def reset_index(status_cb: Callable[[str], None] | None = None) -> None:
 
     status_cb("Creating new empty index...")
     with _using_temp_index(status_cb) as temp_index_name:
-        _configure_index(temp_index_name)
+        _apply_index_settings(temp_index_name, wait=False)
         status_cb("Index recreated!")
     status_cb("Index reset complete.")
 
@@ -429,41 +450,6 @@ def _compare_setting(key, expected):
     )
 
 
-def _apply_settings_with_waits(index_name: str, status_cb: Callable[[str], None] | None = None) -> None:
-    """
-    Apply all index settings and wait for each Meilisearch task to complete.
-
-    Unlike _configure_index() which fires-and-forgets, this function waits for
-    confirmation that each setting has been applied. This is used during reconciliation
-    where we need a definitive state before returning.
-
-    Args:
-        index_name (str): The name of the index to configure.
-        status_cb: Optional callback for status messages.
-    """
-    if status_cb is None:
-        status_cb = log.info
-
-    client = _get_meilisearch_client()
-
-    status_cb(f"Applying distinct attribute to '{index_name}'...")
-    _wait_for_meili_task(client.index(index_name).update_distinct_attribute(INDEX_DISTINCT_ATTRIBUTE))
-
-    status_cb(f"Applying filterable attributes to '{index_name}'...")
-    _wait_for_meili_task(client.index(index_name).update_filterable_attributes(INDEX_FILTERABLE_ATTRIBUTES))
-
-    status_cb(f"Applying searchable attributes to '{index_name}'...")
-    _wait_for_meili_task(client.index(index_name).update_searchable_attributes(INDEX_SEARCHABLE_ATTRIBUTES))
-
-    status_cb(f"Applying sortable attributes to '{index_name}'...")
-    _wait_for_meili_task(client.index(index_name).update_sortable_attributes(INDEX_SORTABLE_ATTRIBUTES))
-
-    status_cb(f"Applying ranking rules to '{index_name}'...")
-    _wait_for_meili_task(client.index(index_name).update_ranking_rules(INDEX_RANKING_RULES))
-
-    status_cb(f"All settings applied to '{index_name}'.")
-
-
 def reconcile_index(
     status_cb: Callable[[str], None] | None = None, warn_cb: Callable[[str], None] | None = None
 ) -> None:  # noqa: E501
@@ -511,7 +497,7 @@ def reconcile_index(
     if drift.is_empty:
         if drift.is_settings_drifted:
             status_cb("Empty index has drifted settings. Reconfiguring...")
-            _apply_settings_with_waits(STUDIO_INDEX_NAME, status_cb)
+            _apply_index_settings(STUDIO_INDEX_NAME, wait=True, status_cb=status_cb)
             status_cb("Reconfigured. Run './manage.py cms' reindex_studio to populate.")
         else:
             status_cb(
@@ -533,7 +519,7 @@ def reconcile_index(
             if match is False:
                 warn_cb(f"  - {field_name}: DRIFTED")
 
-        _apply_settings_with_waits(STUDIO_INDEX_NAME, status_cb)
+        _apply_index_settings(STUDIO_INDEX_NAME, wait=True, status_cb=status_cb)
         warn_cb(
             "Settings applied. Meilisearch will re-index documents in the background. "
             "Consider running './manage.py cms' reindex_studio for a full rebuild "
@@ -624,7 +610,7 @@ def rebuild_index(  # pylint: disable=too-many-statements
         # 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)
+            _apply_index_settings(index_name, wait=False)
 
         ############## Libraries ##############
         status_cb("Indexing libraries...")

openedx/core/djangoapps/content/search/tests/test_reconcile.py

@@ -16,7 +16,7 @@
     from .. import api
     from ..api import (
         IndexDrift,
-        _apply_settings_with_waits,
+        _apply_index_settings,
         _detect_index_drift,
         reconcile_index,
     )
@@ -289,20 +289,20 @@ def test_multiple_mismatches(self, mock_meilisearch):
 @skip_unless_cms
 @override_settings(MEILISEARCH_ENABLED=True)
 @patch("openedx.core.djangoapps.content.search.api.MeilisearchClient")
-class TestApplySettingsWithWaits(TestCase):
-    """Tests for _apply_settings_with_waits()."""
+class TestApplyIndexSettings(TestCase):
+    """Tests for _apply_index_settings()."""
 
     def setUp(self):
         super().setUp()
         api.clear_meilisearch_client()
 
     @patch("openedx.core.djangoapps.content.search.api._wait_for_meili_task", new=MagicMock(return_value=None))
     def test_applies_all_settings(self, mock_meilisearch):
-        """All 5 settings are applied."""
+        """All 5 settings are applied in wait mode."""
         mock_index = mock_meilisearch.return_value.index.return_value
         status_cb = Mock()
 
-        _apply_settings_with_waits("test_index", status_cb)
+        _apply_index_settings("test_index", wait=True, status_cb=status_cb)
 
         mock_index.update_distinct_attribute.assert_called_once_with(INDEX_DISTINCT_ATTRIBUTE)
         mock_index.update_filterable_attributes.assert_called_once_with(INDEX_FILTERABLE_ATTRIBUTES)
@@ -312,27 +312,36 @@ def test_applies_all_settings(self, mock_meilisearch):
 
     @patch("openedx.core.djangoapps.content.search.api._wait_for_meili_task")
     def test_waits_for_each_task(self, mock_wait, mock_meilisearch):
-        """Each settings update is waited on."""
+        """Each settings update is waited on when wait=True."""
         mock_index = mock_meilisearch.return_value.index.return_value
-        # Each update returns a TaskInfo-like object
         task_info = Mock()
         mock_index.update_distinct_attribute.return_value = task_info
         mock_index.update_filterable_attributes.return_value = task_info
         mock_index.update_searchable_attributes.return_value = task_info
         mock_index.update_sortable_attributes.return_value = task_info
         mock_index.update_ranking_rules.return_value = task_info
 
-        _apply_settings_with_waits("test_index")
+        _apply_index_settings("test_index", wait=True)
 
         assert mock_wait.call_count == 5
 
+    @patch("openedx.core.djangoapps.content.search.api._wait_for_meili_task")
+    def test_does_not_wait_when_wait_false(self, mock_wait, mock_meilisearch):
+        """Settings are fire-and-forget when wait=False."""
+        status_cb = Mock()
+
+        _apply_index_settings("test_index", wait=False, status_cb=status_cb)
+
+        mock_wait.assert_not_called()
+        status_cb.assert_called()
+
     @patch("openedx.core.djangoapps.content.search.api._wait_for_meili_task")
     def test_raises_on_task_failure(self, mock_wait, mock_meilisearch):
-        """MeilisearchError is raised if a task fails."""
+        """MeilisearchError is raised if a waited-on task fails."""
         mock_wait.side_effect = MeilisearchError("Task failed")
 
         with pytest.raises(MeilisearchError):
-            _apply_settings_with_waits("test_index")
+            _apply_index_settings("test_index", wait=True)
 
 
 @skip_unless_cms
@@ -380,7 +389,7 @@ def test_index_empty_configured(self, mock_drift, mock_meilisearch):
         )
 
     @patch("openedx.core.djangoapps.content.search.api._detect_index_drift")
-    @patch("openedx.core.djangoapps.content.search.api._apply_settings_with_waits")
+    @patch("openedx.core.djangoapps.content.search.api._apply_index_settings")
     def test_index_empty_drifted_settings(self, mock_apply, mock_drift, mock_meilisearch):
         """When index is empty and settings drifted, settings are applied."""
         mock_drift.return_value = IndexDrift(
@@ -397,7 +406,7 @@ def test_index_empty_drifted_settings(self, mock_apply, mock_drift, mock_meilise
 
         reconcile_index(status_cb=status_cb)
 
-        mock_apply.assert_called_once()
+        mock_apply.assert_called_once_with(api.STUDIO_INDEX_NAME, wait=True, status_cb=status_cb)
         status_cb.assert_any_call("Empty index has drifted settings. Reconfiguring...")
 
     @patch("openedx.core.djangoapps.content.search.api._detect_index_drift")
@@ -441,7 +450,7 @@ def test_index_populated_configured(self, mock_drift, mock_meilisearch):
         status_cb.assert_any_call("Index is populated and correctly configured. No action needed.")
 
     @patch("openedx.core.djangoapps.content.search.api._detect_index_drift")
-    @patch("openedx.core.djangoapps.content.search.api._apply_settings_with_waits")
+    @patch("openedx.core.djangoapps.content.search.api._apply_index_settings")
     def test_index_populated_drifted_settings(self, mock_apply, mock_drift, mock_meilisearch):
         """When index is populated and drifted, settings are applied with warnings."""
         mock_drift.return_value = IndexDrift(
@@ -459,7 +468,7 @@ def test_index_populated_drifted_settings(self, mock_apply, mock_drift, mock_mei
 
         reconcile_index(status_cb=status_cb, warn_cb=warn_cb)
 
-        mock_apply.assert_called_once()
+        mock_apply.assert_called_once_with(api.STUDIO_INDEX_NAME, wait=True, status_cb=status_cb)
         # Check that drifted fields are logged
         warn_cb.assert_any_call("  - filterableAttributes: DRIFTED")
         warn_cb.assert_any_call("  - searchableAttributes: DRIFTED")