Repository: DataBiosphere/getm (language: python)

Issue:
CLI argument to halt batch downloads eagerly on error
The default behavior is complete as many downloads as possible. However, some users may wish to save time and exit early.


Discussion / hints:

Fix patch:
diff --git a/getm/cli.py b/getm/cli.py
index 840047d..7c0c39d 100644
--- a/getm/cli.py
+++ b/getm/cli.py
@@ -27,6 +27,7 @@
 
 class CLI:
     exit_code = 0
+    continue_after_error = False
 
     @classmethod
     def exit(cls, code: Optional[int]=None):
@@ -48,11 +49,15 @@ def log_warning(cls, **kwargs):
     def log_error(cls, **kwargs):
         cls.exit_code = 1
         logger.error(json.dumps(kwargs))
+        if not cls.continue_after_error:
+            cls.exit()
 
     @classmethod
     def log_exception(cls, **kwargs):
         cls.exit_code = 1
         logger.exception(json.dumps(kwargs))
+        if not cls.continue_after_error:
+            cls.exit()
 
 def checksum_for_url(url: str) -> Optional[GETMChecksum]:
     """Probe headers for checksum information, return or None."""
@@ -240,26 +245,32 @@ def parse_args(cli_args: Optional[List[str]]=None) -> argparse.Namespace:
     parser.add_argument("--multipart-threshold",
                         default=default_chunk_size,
                         help="multipart threshold")
+    parser.add_argument("--continue-after-error",
+                        "-c",
+                        action="store_true",
+                        help=("Continue downloading files if an error occurs."
+                              "Exit status is non-zero if any downloads fail."))
     args = parser.parse_args(args=cli_args)
     if not (args.url or args.manifest) or (args.url and args.manifest):
         parser.print_usage()
         CLI.log_error(message="One of 'url' or '--manifest' must be specified, but not both.")
-        CLI.exit(1)
     if not (1 <= args.multipart_concurrency <= 2):
         parser.print_usage()
         CLI.log_error(message="'--multipart_concurrency' must be in the range [1,2], inclusive.")
-        CLI.exit(1)
     return args
 
-def main():
-    """This is the main CLI entry point."""
-    multiprocessing.set_start_method("fork")
-    args = parse_args()
-
+def config_cli(args: argparse.Namespace):
     if args.vv:
         logger.setLevel(logging.DEBUG)
     elif args.v:
         logger.setLevel(logging.INFO)
+    CLI.continue_after_error = args.continue_after_error
+
+def main():
+    """This is the main CLI entry point."""
+    multiprocessing.set_start_method("fork")
+    args = parse_args()
+    config_cli(args)
 
     if args.url:
         info = dict(url=args.url)
@@ -280,3 +291,5 @@ def main():
 
     _validate_manifest(manifest)
     download(manifest, args.oneshot_concurrency, args.multipart_concurrency, args.multipart_threshold)
+    if CLI.exit_code:
+        CLI.exit()


Test patch:
diff --git a/tests/test_cli.py b/tests/test_cli.py
index ade1745..a478d89 100644
--- a/tests/test_cli.py
+++ b/tests/test_cli.py
@@ -89,6 +89,8 @@ def test_cli_args_and_config(self, *args):
             ("can't supply positional with manifest", "-m manifest.json https://no/wrong".split()),
             ("multipart concurrencty to high", "-m manifest.json --multipart-concurrency 3".split()),
             ("multipart concurrencty to low", "-m manifest.json --multipart-concurrency 0".split()),
+            ("continue should not work yet 1", "-c -m manifest.json http//doom".split()),
+            ("continue should not work yet 2", "-c".split()),
         ]
 
         for msg, arglist in error_tests:
@@ -99,6 +101,12 @@ def test_cli_args_and_config(self, *args):
                 self.assertEqual(1, cli.CLI.exit_code)
                 self.assertEqual(1, cm.exception.code)
 
+        with self.subTest("continue after error"):
+            self.assertEqual(cli.CLI.continue_after_error, False)
+            args = cli.parse_args("-m manifest.json --continue-after-error".split())
+            cli.config_cli(args)
+            self.assertEqual(cli.CLI.continue_after_error, True)
+
     def test_download(self, *args):
         multipart_threshold = 7
         oneshot_sizes = 3 * [1]