modal shell takes container IDs now (#2336)

* Allow `modal shell` into a container ID

* Fix cmd parsing

modal/cli/run.py

@@ -331,49 +331,46 @@ def serve(
 
 
 def shell(
-    func_ref: Optional[str] = typer.Argument(
+    container_or_function: Optional[str] = typer.Argument(
         default=None,
         help=(
-            "Path to a Python file with an App or Modal function with container parameters."
-            " Can also include a function specifier, like `module.py::func`, when the file defines multiple functions."
+            "ID of running container, or path to a Python file containing a Modal App."
+            " Can also include a function specifier, like `module.py::func`, if the file defines multiple functions."
         ),
-        metavar="FUNC_REF",
+        metavar="REF",
     ),
     cmd: str = typer.Option("/bin/bash", "-c", "--cmd", help="Command to run inside the Modal image."),
     env: str = ENV_OPTION,
     image: Optional[str] = typer.Option(
-        default=None, help="Container image tag for inside the shell (if not using FUNC_REF)."
+        default=None, help="Container image tag for inside the shell (if not using REF)."
     ),
-    add_python: Optional[str] = typer.Option(default=None, help="Add Python to the image (if not using FUNC_REF)."),
+    add_python: Optional[str] = typer.Option(default=None, help="Add Python to the image (if not using REF)."),
     volume: Optional[typing.List[str]] = typer.Option(
         default=None,
         help=(
-            "Name of a `modal.Volume` to mount inside the shell at `/mnt/{name}` (if not using FUNC_REF)."
+            "Name of a `modal.Volume` to mount inside the shell at `/mnt/{name}` (if not using REF)."
             " Can be used multiple times."
         ),
     ),
-    cpu: Optional[int] = typer.Option(
-        default=None, help="Number of CPUs to allocate to the shell (if not using FUNC_REF)."
-    ),
+    cpu: Optional[int] = typer.Option(default=None, help="Number of CPUs to allocate to the shell (if not using REF)."),
     memory: Optional[int] = typer.Option(
-        default=None, help="Memory to allocate for the shell, in MiB (if not using FUNC_REF)."
+        default=None, help="Memory to allocate for the shell, in MiB (if not using REF)."
     ),
     gpu: Optional[str] = typer.Option(
         default=None,
-        help="GPUs to request for the shell, if any. Examples are `any`, `a10g`, `a100:4` (if not using FUNC_REF).",
+        help="GPUs to request for the shell, if any. Examples are `any`, `a10g`, `a100:4` (if not using REF).",
     ),
     cloud: Optional[str] = typer.Option(
         default=None,
         help=(
-            "Cloud provider to run the shell on. "
-            "Possible values are `aws`, `gcp`, `oci`, `auto` (if not using FUNC_REF)."
+            "Cloud provider to run the shell on. " "Possible values are `aws`, `gcp`, `oci`, `auto` (if not using REF)."
         ),
     ),
     region: Optional[str] = typer.Option(
         default=None,
         help=(
-            "Region(s) to run the shell on. "
-            "Can be a single region or a comma-separated list to choose from (if not using FUNC_REF)."
+            "Region(s) to run the container on. "
+            "Can be a single region or a comma-separated list to choose from (if not using REF)."
         ),
     ),
 ):
@@ -410,8 +407,21 @@ def shell(
 
     app = App("modal shell")
 
-    if func_ref is not None:
-        function = import_function(func_ref, accept_local_entrypoint=False, accept_webhook=True, base_cmd="modal shell")
+    if container_or_function is not None:
+        # `modal shell` with a container ID is a special case, alias for `modal container exec`.
+        if (
+            container_or_function.startswith("ta-")
+            and len(container_or_function[3:]) > 0
+            and container_or_function[3:].isalnum()
+        ):
+            from .container import exec
+
+            exec(container_id=container_or_function, command=shlex.split(cmd), pty=True)
+            return
+
+        function = import_function(
+            container_or_function, accept_local_entrypoint=False, accept_webhook=True, base_cmd="modal shell"
+        )
         assert isinstance(function, Function)
         function_spec: _FunctionSpec = function.spec
         start_shell = partial(

modal/serving.py

@@ -1,7 +1,6 @@
 # Copyright Modal Labs 2023
 import multiprocessing
 import platform
-import sys
 from multiprocessing.context import SpawnProcess
 from multiprocessing.synchronize import Event
 from typing import TYPE_CHECKING, AsyncGenerator, Optional, Set, TypeVar
@@ -88,16 +87,6 @@ async def _run_watch_loop(
             await _terminate(curr_proc)
 
 
-def _get_clean_app_description(app_ref: str) -> str:
-    # If possible, consider the 'ref' argument the start of the app's args. Everything
-    # before it Modal CLI cruft (eg. `modal serve --timeout 1.0`).
-    try:
-        func_ref_arg_idx = sys.argv.index(app_ref)
-        return " ".join(sys.argv[func_ref_arg_idx:])
-    except ValueError:
-        return " ".join(sys.argv)
-
-
 @asynccontextmanager
 async def _serve_app(
     app: "_App",