Merge pull request #656 from openedx/pwnage101/ENT-7311-8

re-add constant-memory code and add "feat: do not index objects that are not indexable"

enterprise_catalog/apps/api_client/algolia.py

@@ -91,7 +91,7 @@ def index_exists(self):
 
         return exists
 
-    def replace_all_objects(self, algolia_objects):
+    def replace_all_objects(self, algolia_objects):  # pragma: no cover
         """
         Clears all objects from the index and replaces them with a new set of objects. The records are
         replaced in the index without any downtime due to an atomic reindex.
@@ -109,15 +109,10 @@ def replace_all_objects(self, algolia_objects):
             self.algolia_index.replace_all_objects(algolia_objects, {
                 'safe': True,  # wait for asynchronous indexing operations to complete
             })
-            logger.info(
-                'The %s Algolia index was successfully indexed with %s objects.',
-                self.ALGOLIA_INDEX_NAME,
-                len(algolia_objects),
-            )
+            logger.info('The %s Algolia index was successfully indexed.', self.ALGOLIA_INDEX_NAME)
         except AlgoliaException as exc:
             logger.exception(
-                'Could not index %s objects in the %s Algolia index due to an exception.',
-                len(algolia_objects),
+                'Could not index objects in the %s Algolia index due to an exception.',
                 self.ALGOLIA_INDEX_NAME,
             )
             raise exc

enterprise_catalog/apps/catalog/management/commands/reindex_algolia.py

@@ -17,11 +17,18 @@ class Command(BaseCommand):
     )
 
     def add_arguments(self, parser):
-        # Argument to force execution of celery task, ignoring time since last execution
         parser.add_argument(
             '--force',
             default=False,
             action='store_true',
+            help='Force execution of celery task, ignoring time since last execution.',
+        )
+        parser.add_argument(
+            '--dry-run',
+            dest='dry_run',
+            action='store_true',
+            default=False,
+            help='Generate algolia products to index, but do not actually send them to algolia for indexing.',
         )
 
     def handle(self, *args, **options):
@@ -31,11 +38,13 @@ def handle(self, *args, **options):
         options.update(CatalogUpdateCommandConfig.current_options())
         try:
             force_task_execution = options.get('force', False)
+            dry_run = options.get('dry_run', False)
             if options.get('no_async'):
-                index_enterprise_catalog_in_algolia_task()
+                # For some reason in order to call a celery task in-memory you must pass kwargs as args.
+                index_enterprise_catalog_in_algolia_task(force_task_execution, dry_run)
             else:
                 index_enterprise_catalog_in_algolia_task.apply_async(
-                    kwargs={'force': force_task_execution}
+                    kwargs={'force': force_task_execution, 'dry_run': dry_run}
                 ).get()
             logger.info(
                 'index_enterprise_catalog_in_algolia_task from command reindex_algolia finished successfully.'

enterprise_catalog/apps/catalog/management/commands/tests/test_reindex_algolia.py

@@ -20,28 +20,44 @@ def setUpTestData(cls):
         super().setUpTestData()
         cls.content_metadata = ContentMetadataFactory.create_batch(3, content_type=COURSE)
 
-    def setUp(self):
-        super().setUp()
-        self.command_config_mock = mock.patch('enterprise_catalog.apps.catalog.models.CatalogUpdateCommandConfig')
-        mock_config = self.command_config_mock.start()
-        mock_config.current_config.return_value = {
+    @mock.patch(PATH_PREFIX + 'index_enterprise_catalog_in_algolia_task')
+    @mock.patch('enterprise_catalog.apps.catalog.management.commands.reindex_algolia.CatalogUpdateCommandConfig')
+    def test_reindex_algolia(self, mock_command_config, mock_task):
+        """
+        Verify that the job spins off the correct number of index_enterprise_catalog_in_algolia_task
+        """
+        mock_command_config.current_options.return_value = {
             'force': False,
             'no_async': False,
         }
+        call_command(self.command_name)
+        mock_task.apply_async.assert_called_once_with(kwargs={'force': False, 'dry_run': False})
+        mock_task.apply_async.return_value.get.assert_called_once_with()
 
-    def tearDown(self):
-        super().tearDown()
-        self.command_config_mock.stop()
+    @mock.patch(PATH_PREFIX + 'index_enterprise_catalog_in_algolia_task')
+    @mock.patch('enterprise_catalog.apps.catalog.management.commands.reindex_algolia.CatalogUpdateCommandConfig')
+    def test_reindex_algolia_no_async(self, mock_command_config, mock_task):
+        """
+        Verify that the job spins off the correct number of index_enterprise_catalog_in_algolia_task
+        """
+        mock_command_config.current_options.return_value = {
+            'force': False,
+            'no_async': True,
+        }
+        call_command(self.command_name)
+        mock_task.assert_called_once_with(False, False)  # force=False, dry_run=False
+        mock_task.apply_async.assert_not_called()
 
     @mock.patch(PATH_PREFIX + 'index_enterprise_catalog_in_algolia_task')
-    @mock.patch('enterprise_catalog.apps.catalog.models.CatalogUpdateCommandConfig')
-    def test_reindex_algolia(self, mock_command_config, mock_task):
+    @mock.patch('enterprise_catalog.apps.catalog.management.commands.reindex_algolia.CatalogUpdateCommandConfig')
+    def test_reindex_algolia_dry_run(self, mock_command_config, mock_task):
         """
         Verify that the job spins off the correct number of index_enterprise_catalog_in_algolia_task
         """
         mock_command_config.current_options.return_value = {
             'force': False,
             'no_async': False,
         }
-        call_command(self.command_name)
+        call_command(self.command_name, dry_run=True)
+        mock_task.apply_async.assert_called_once_with(kwargs={'force': False, 'dry_run': True})
         mock_task.apply_async.return_value.get.assert_called_once_with()

enterprise_catalog/apps/catalog/tests/factories.py

@@ -75,6 +75,7 @@ class Meta:
 
     # model fields
     content_key = factory.Sequence(lambda n: f'{str(n).zfill(5)}_metadata_item')
+    content_uuid = factory.LazyFunction(uuid4)
     content_type = factory.Iterator([COURSE_RUN, COURSE, PROGRAM, LEARNER_PATHWAY])
     parent_content_key = None
 
@@ -83,7 +84,7 @@ def json_metadata(self):
         json_metadata = {
             'key': self.content_key,
             'aggregation_key': f'{self.content_type}:{self.content_key}',
-            'uuid': str(uuid4()),
+            'uuid': str(self.content_uuid),
             'title': self.title,
         }
         if self.content_type == COURSE:

pylintrc

@@ -2,7 +2,7 @@
 # ** DO NOT EDIT THIS FILE **
 # ***************************
 #
-# This file was generated by edx-lint: https://github.com/edx/edx-lint
+# This file was generated by edx-lint: https://github.com/openedx/edx-lint
 #
 # If you want to change this file, you have two choices, depending on whether
 # you want to make a local change that applies only to this repo, or whether
@@ -28,7 +28,7 @@
 # CENTRAL CHANGE:
 #
 # 1. Edit the pylintrc file in the edx-lint repo at
-#    https://github.com/edx/edx-lint/blob/master/edx_lint/files/pylintrc
+#    https://github.com/openedx/edx-lint/blob/master/edx_lint/files/pylintrc
 #
 # 2. install the updated version of edx-lint (in edx-lint):
 #
@@ -64,7 +64,7 @@
 # SERIOUSLY.
 #
 # ------------------------------
-# Generated by edx-lint version: 5.2.4
+# Generated by edx-lint version: 5.2.5
 # ------------------------------
 [MASTER]
 ignore = ,migrations, settings, wsgi.py
@@ -285,7 +285,7 @@ disable =
 	feature-toggle-needs-doc,
 	illegal-waffle-usage,
 
-	logging-fstring-interpolation,,invalid-name,missing-docstring,consider-using-f-string,logging-format-interpolation,useless-option-value,unknown-option-value
+	logging-fstring-interpolation,,invalid-name,missing-docstring,consider-using-f-string,logging-format-interpolation,useless-option-value,unknown-option-value,consider-using-dict-items,consider-iterating-dictionary
 
 [REPORTS]
 output-format = text
@@ -382,4 +382,4 @@ int-import-graph =
 [EXCEPTIONS]
 overgeneral-exceptions = Exception
 
-# 8d3e98bf77c189c1b91198801398e9f3e410fdc5
+# b86606bb77e6a87fcf5f6893592d35d432a507f5

pylintrc_tweaks

@@ -5,4 +5,4 @@ ignore+= ,migrations, settings, wsgi.py
 const-rgx = (([A-Z_][A-Z0-9_]*)|(__.*__)|log|urlpatterns|logger|User)$
 
 [MESSAGES CONTROL]
-DISABLE+= ,invalid-name,missing-docstring,consider-using-f-string,logging-format-interpolation,useless-option-value,unknown-option-value
+DISABLE+= ,invalid-name,missing-docstring,consider-using-f-string,logging-format-interpolation,useless-option-value,unknown-option-value,consider-using-dict-items,consider-iterating-dictionary