Add importcontent --error-on-fail option

Normally importcontent carries on when it can't import a file and prints
a warning on the number of skipped files. That makes it unreliable when
trying to ensure a fully provisioned device. Add the --error-on-fail
option to make import errors fatal.

Fixes: learningequality#9258

kolibri/core/content/management/commands/importcontent.py

@@ -110,6 +110,14 @@ def add_arguments(self, parser):
             help="Import all updated content after a channel version upgrade",
         )
 
+        parser.add_argument(
+            "--error-on-fail",
+            action="store_true",
+            default=False,
+            dest="error_on_fail",
+            help="Raise an error when a file has failed to be imported",
+        )
+
         # to implement these two groups of commands and their corresponding
         # arguments, we'll need argparse.subparsers.
         subparsers = parser.add_subparsers(
@@ -158,6 +166,7 @@ def download_content(
         peer_id=None,
         renderable_only=True,
         import_updates=False,
+        error_on_fail=False,
     ):
         self._transfer(
             DOWNLOAD_METHOD,
@@ -168,6 +177,7 @@ def download_content(
             peer_id=peer_id,
             renderable_only=renderable_only,
             import_updates=import_updates,
+            error_on_fail=error_on_fail,
         )
 
     def copy_content(
@@ -179,6 +189,7 @@ def copy_content(
         exclude_node_ids=None,
         renderable_only=True,
         import_updates=False,
+        error_on_fail=False,
     ):
         self._transfer(
             COPY_METHOD,
@@ -189,6 +200,7 @@ def copy_content(
             exclude_node_ids=exclude_node_ids,
             renderable_only=renderable_only,
             import_updates=import_updates,
+            error_on_fail=error_on_fail,
         )
 
     def _transfer(  # noqa: max-complexity=16
@@ -203,6 +215,7 @@ def _transfer(  # noqa: max-complexity=16
         peer_id=None,
         renderable_only=True,
         import_updates=False,
+        error_on_fail=False,
     ):
         try:
             if not import_updates:
@@ -389,8 +402,10 @@ def _transfer(  # noqa: max-complexity=16
                             logger.error(
                                 "An error occurred during content import: {}".format(e)
                             )
+
                             if (
-                                isinstance(e, requests.exceptions.HTTPError)
+                                not error_on_fail
+                                and isinstance(e, requests.exceptions.HTTPError)
                                 and e.response.status_code == 404
                             ) or (isinstance(e, OSError) and e.errno == 2):
                                 # Continue file import when the current file is not found from the source and is skipped.
@@ -491,6 +506,7 @@ def handle_async(self, *args, **options):
                 peer_id=options["peer_id"],
                 renderable_only=options["renderable_only"],
                 import_updates=options["import_updates"],
+                error_on_fail=options["error_on_fail"],
             )
         elif options["command"] == "disk":
             self.copy_content(
@@ -501,6 +517,7 @@ def handle_async(self, *args, **options):
                 exclude_node_ids=options["exclude_node_ids"],
                 renderable_only=options["renderable_only"],
                 import_updates=options["import_updates"],
+                error_on_fail=options["error_on_fail"],
             )
         else:
             self._parser.print_help()

kolibri/core/content/test/test_import_export.py

@@ -1118,6 +1118,63 @@ def test_remote_import_file_compressed_on_gcs(
                 public=False,
             )
 
+    @patch("kolibri.core.content.management.commands.importcontent.logger.warning")
+    @patch(
+        "kolibri.core.content.management.commands.importcontent.paths.get_content_storage_file_path"
+    )
+    def test_remote_import_error_on_fail(
+        self,
+        path_mock,
+        logger_mock,
+        annotation_mock,
+        get_import_export_mock,
+        channel_list_status_mock,
+    ):
+        fd1, local_dest_path_1 = tempfile.mkstemp()
+        fd2, local_dest_path_2 = tempfile.mkstemp()
+        fd3, local_dest_path_3 = tempfile.mkstemp()
+        os.close(fd1)
+        os.close(fd2)
+        os.close(fd3)
+        path_mock.side_effect = [
+            local_dest_path_1,
+            local_dest_path_2,
+            local_dest_path_3,
+        ]
+        ContentNode.objects.filter(pk="2b6926ed22025518a8b9da91745b51d3").update(
+            available=False
+        )
+        LocalFile.objects.filter(
+            files__contentnode__pk="2b6926ed22025518a8b9da91745b51d3"
+        ).update(file_size=1, available=False)
+        get_import_export_mock.return_value = (
+            1,
+            list(
+                LocalFile.objects.filter(
+                    files__contentnode__pk="2b6926ed22025518a8b9da91745b51d3"
+                ).values("id", "file_size", "extension")
+            ),
+            10,
+        )
+
+        node_id = ["2b6926ed22025518a8b9da91745b51d3"]
+        with self.assertRaises(HTTPError):
+            call_command(
+                "importcontent",
+                "network",
+                self.the_channel_id,
+                node_ids=node_id,
+                renderable_only=False,
+                error_on_fail=True,
+            )
+        annotation_mock.set_content_visibility.assert_called_with(
+            self.the_channel_id,
+            [],
+            node_ids=node_id,
+            exclude_node_ids=None,
+            public=False,
+        )
+
 
 @override_option("Paths", "CONTENT_DIR", tempfile.mkdtemp())
 class ExportChannelTestCase(TestCase):