Merge branch 'trs/build/no-download-no-build-dir-no-problem'

CHANGES.md

@@ -13,6 +13,15 @@ development source code and as such may not be routinely kept up to date.
 
 # __NEXT__
 
+## Improvements
+
+* build: Providing a path to a pathogen build directory is no longer required
+  when the AWS Batch runtime is in use (e.g. with `--aws-batch`) and both the
+  `--attach` and `--no-download` options are given.  This allows usages which
+  just want to check job status or logs to stop providing a meaningless/unused
+  directory.
+  ([#305](https://github.com/nextstrain/cli/pull/305))
+
 
 # 7.2.0 (17 August 2023)
 

doc/commands/build.rst

@@ -39,7 +39,7 @@ positional arguments
 
 .. option:: <directory>
 
-    Path to pathogen build directory
+    Path to pathogen build directory.  Required, except when the AWS Batch runtime is in use and both --attach and --no-download are given.  
 
 .. option:: ...
 

nextstrain/cli/__init__.py

@@ -19,7 +19,7 @@
 from .argparse    import HelpFormatter, register_commands, register_default_command
 from .command     import build, view, deploy, remote, shell, update, setup, check_setup, login, logout, whoami, version, init_shell, authorization, debugger
 from .debug       import DEBUGGING
-from .errors      import NextstrainCliError
+from .errors      import NextstrainCliError, UsageError
 from .util        import warn
 from .__version__ import __version__ # noqa: F401 (for re-export)
 
@@ -36,11 +36,18 @@ def run(args):
         return opts.__command__.run(opts)
 
     except NextstrainCliError as error:
+        exit_status = 1
+
         if DEBUGGING:
             traceback.print_exc()
         else:
+            if isinstance(error, UsageError):
+                warn(opts.__parser__.format_usage())
+                exit_status = 2
+
             warn(error)
-        return 1
+
+        return exit_status
 
     except AssertionError:
         traceback.print_exc()

nextstrain/cli/argparse.py

@@ -78,7 +78,18 @@ def register_commands(parser, commands):
 
     for cmd in commands:
         subparser = cmd.register_parser(subparsers)
-        subparser.set_defaults( __command__ = cmd )
+
+        # No need to worry about reference cycles of subparser to itself.
+        # CPython's GC uses automatic reference counting complemented with
+        # traversal-based reachability detection that can handle cycles.¹
+        # Other implementations may not, but we largely don't care about those,
+        # and taking a big step back, our processes here just don't live long
+        # enough for the cycle to be a memory issue (plus it's just one cycle,
+        # not recreated over and over again in a hot loop).
+        #   -trs, 24 August 2023
+        #
+        # ¹ <https://docs.python.org/3.11/reference/datamodel.html#index-2>
+        subparser.set_defaults( __command__ = cmd, __parser__ = subparser )
 
         # Ensure all subparsers format like the top-level parser
         subparser.formatter_class = parser.formatter_class

nextstrain/cli/command/build.py

@@ -20,6 +20,7 @@
 from textwrap import dedent
 from .. import runner
 from ..argparse import add_extended_help_flags, AppendOverwriteDefault, SKIP_AUTO_DEFAULT_IN_HELP
+from ..errors import UsageError, UserError
 from ..util import byte_quantity, warn
 from ..volume import store_volume
 
@@ -118,9 +119,12 @@ def register_parser(subparser):
     # Positional parameters
     parser.add_argument(
         "directory",
-        help    = "Path to pathogen build directory",
+        help    = "Path to pathogen build directory.  "
+                  "Required, except when the AWS Batch runtime is in use and both --attach and --no-download are given.  "
+                  f"{SKIP_AUTO_DEFAULT_IN_HELP}",
         metavar = "<directory>",
-        action  = store_volume("build"))
+        action  = store_volume("build"),
+        nargs   = "?")
 
     # Register runner flags and arguments
     runner.register_runners(parser, exec = ["snakemake", "--printshellcmds", ...])
@@ -129,8 +133,25 @@ def register_parser(subparser):
 
 
 def run(opts):
+    # We must check this before the conditions under which opts.build is
+    # optional because otherwise we could pass a missing build dir to a runner
+    # which ignores opts.attach.
+    if (opts.attach or opts.detach) and opts.__runner__ is not runner.aws_batch:
+        raise UserError(f"""
+            The --attach/--detach options are only supported when using the AWS
+            Batch runtime.  Did you forget to specify --aws-batch?
+            """)
+
     # Ensure our build dir exists
-    if not opts.build.src.is_dir():
+    if opts.build is None:
+        if opts.attach and not opts.download:
+            # Don't require a build directory with --attach + --no-download.
+            # User just wants to check status and/or logs.
+            pass
+        else:
+            raise UsageError("Path to a pathogen build <directory> is required.")
+
+    elif not opts.build.src.is_dir():
         warn("Error: Build path \"%s\" does not exist or is not a directory." % opts.build.src)
 
         if not opts.build.src.is_absolute():

nextstrain/cli/errors.py

@@ -18,3 +18,9 @@ def __init__(self, message, *args, **kwargs):
         formatted_message = dedent(message.lstrip("\n").rstrip()).format(*args, **kwargs)
 
         super().__init__("Error: " + formatted_message)
+
+class UsageError(UserError):
+    """
+    Prints brief command usage before the error message.
+    """
+    pass

nextstrain/cli/runner/aws_batch/__init__.py

@@ -85,7 +85,7 @@
 from sys import exit
 from textwrap import dedent
 from time import sleep, time
-from typing import Iterable
+from typing import Iterable, Optional
 from uuid import uuid4
 from ...types import Env, RunnerSetupStatus, RunnerTestResults, RunnerUpdateStatus
 from ...util import colored, warn
@@ -164,13 +164,13 @@ def register_arguments(parser) -> None:
 
 def run(opts, argv, working_volume = None, extra_env: Env = {}, cpus: int = None, memory: int = None) -> int:
     # Unlike other runners, the AWS Bach runner currently *requires* a working
-    # dir.  This is ok as we only provide the AWS Batch runner for commands
-    # which also require a working dir (e.g. build), whereas other runners also
-    # work with commands that don't.
-    #   -trs, 28 Feb 2022
-    assert working_volume is not None
+    # dir in most usages.  This is ok as we only provide the AWS Batch runner
+    # for commands which also require a working dir (e.g. build), whereas other
+    # runners also work with commands that don't.
+    #   -trs, 28 Feb 2022 (updated 24 August 2023)
+    assert working_volume is not None or (opts.attach and not opts.download)
 
-    local_workdir = working_volume.src.resolve(strict = True)
+    local_workdir = working_volume.src.resolve(strict = True) if working_volume else None
 
     if opts.attach:
         print_stage("Attaching to Nextstrain AWS Batch Job ID:", opts.attach)
@@ -191,6 +191,8 @@ def run(opts, argv, working_volume = None, extra_env: Env = {}, cpus: int = None
 
         print_stage("Job is %s" % job.status)
     else:
+        assert local_workdir is not None
+
         # Generate our own unique run id since we can't know the AWS Batch job id
         # until we submit it.  This run id is used for workdir and run results
         # storage on S3, in a bucket accessible to both Batch jobs and CLI users.
@@ -357,6 +359,8 @@ def handler(sig, frame):
 
     # Download results if we didn't stop the job early.
     if opts.download and not stop_sent and not job.stopped:
+        assert local_workdir is not None
+
         patterns = opts.download if isinstance(opts.download, list) else None
 
         if patterns:
@@ -378,7 +382,7 @@ def handler(sig, frame):
     )
 
 
-def detach(job: jobs.JobState, local_workdir: Path) -> int:
+def detach(job: jobs.JobState, local_workdir: Optional[Path]) -> int:
     """
     Detach from the specified *job* and print a message about how to re-attach
     (using the *local_workdir*).
@@ -393,7 +397,7 @@ def detach(job: jobs.JobState, local_workdir: Path) -> int:
         "--attach", shlex.quote(job.id),
 
         # Preserve the local workdir, which has been resolved to an absolute path
-        shlex.quote(str(local_workdir))
+        shlex.quote(str(local_workdir) if local_workdir else ".")
     ])
 
     print(dedent("""

nextstrain/cli/volume.py

@@ -34,7 +34,7 @@ class store(argparse.Action):
         def __call__(self, parser, namespace, values, option_strings = None):
             # Add the new volume to the list of volumes
             volumes    = getattr(namespace, "volumes", [])
-            new_volume = NamedVolume(volume_name, Path(values))
+            new_volume = NamedVolume(volume_name, Path(values)) if values else None
             setattr(namespace, "volumes", [*volumes, new_volume])
 
             # Allow the new volume to be found by name on the opts object