Fixes issue where modal run of a method on a class triggered deprec…

…ation warning [CLI-304] (#2761)

modal/app.py

@@ -507,7 +507,7 @@ def registered_functions(self) -> dict[str, _Function]:
         return self._functions
 
     @property
-    def registered_classes(self) -> dict[str, _Function]:
+    def registered_classes(self) -> dict[str, _Cls]:
         """All modal.Cls objects registered on the app."""
         return self._classes
 

modal/cli/import_refs.py

@@ -9,8 +9,11 @@
 
 import dataclasses
 import importlib
+import importlib.util
 import inspect
 import sys
+import types
+from dataclasses import dataclass
 from pathlib import Path
 from typing import Any, Optional, Union
 
@@ -19,13 +22,20 @@
 from rich.markdown import Markdown
 
 from modal.app import App, LocalEntrypoint
+from modal.cls import Cls
 from modal.exception import InvalidError, _CliUserExecutionError
 from modal.functions import Function
 
 
 @dataclasses.dataclass
 class ImportRef:
     file_or_module: str
+
+    # object_path is a .-delimited path to the object to execute, or a parent from which to infer the object
+    # e.g.
+    # function or local_entrypoint in module scope
+    # app in module scope [+ method name]
+    # app [+ function/entrypoint on that app]
     object_path: Optional[str]
 
 
@@ -62,11 +72,14 @@ def import_file_or_module(file_or_module: str):
         sys.path.insert(0, str(full_path.parent))
 
         module_name = inspect.getmodulename(file_or_module)
+        assert module_name is not None
         # Import the module - see https://docs.python.org/3/library/importlib.html#importing-a-source-file-directly
         spec = importlib.util.spec_from_file_location(module_name, file_or_module)
+        assert spec is not None
         module = importlib.util.module_from_spec(spec)
         sys.modules[module_name] = module
         try:
+            assert spec.loader
             spec.loader.exec_module(module)
         except Exception as exc:
             raise _CliUserExecutionError(str(full_path)) from exc
@@ -79,23 +92,39 @@ def import_file_or_module(file_or_module: str):
     return module
 
 
-def get_by_object_path(obj: Any, obj_path: str) -> Optional[Any]:
+@dataclass
+class MethodReference:
+    """This helps with deferring method reference until after the class gets instantiated by the CLI"""
+
+    cls: Cls
+    method_name: str
+
+
+def get_by_object_path(obj: Any, obj_path: str) -> Union[Function, LocalEntrypoint, MethodReference, App, None]:
     # Try to evaluate a `.`-delimited object path in a Modal context
     # With the caveat that some object names can actually have `.` in their name (lifecycled methods' tags)
 
     # Note: this is eager, so no backtracking is performed in case an
     # earlier match fails at some later point in the path expansion
     prefix = ""
-    for segment in obj_path.split("."):
+    obj_path_segments = obj_path.split(".")
+    for i, segment in enumerate(obj_path_segments):
         attr = prefix + segment
+        if isinstance(obj, App):
+            if attr in obj.registered_entrypoints:
+                # local entrypoints can't be accessed via getattr
+                obj = obj.registered_entrypoints[attr]
+                continue
+        if isinstance(obj, Cls):
+            remaining_segments = obj_path_segments[i:]
+            remaining_path = ".".join(remaining_segments)
+            if len(remaining_segments) > 1:
+                raise ValueError(f"{obj._get_name()} is a class, but {remaining_path} is not a method reference")
+            # TODO: add method inference here?
+            return MethodReference(obj, remaining_path)
+
         try:
-            if isinstance(obj, App):
-                if attr in obj.registered_entrypoints:
-                    # local entrypoints are not on stub blueprint
-                    obj = obj.registered_entrypoints[attr]
-                    continue
             obj = getattr(obj, attr)
-
         except Exception:
             prefix = f"{prefix}{segment}."
         else:
@@ -109,54 +138,62 @@ def get_by_object_path(obj: Any, obj_path: str) -> Optional[Any]:
 
 def _infer_function_or_help(
     app: App, module, accept_local_entrypoint: bool, accept_webhook: bool
-) -> Union[Function, LocalEntrypoint]:
-    function_choices = set(app.registered_functions)
-    if not accept_webhook:
-        function_choices -= set(app.registered_web_endpoints)
-    if accept_local_entrypoint:
-        function_choices |= set(app.registered_entrypoints.keys())
+) -> Union[Function, LocalEntrypoint, MethodReference]:
+    """Using only an app - automatically infer a single "runnable" for a `modal run` invocation
 
-    sorted_function_choices = sorted(function_choices)
-    registered_functions_str = "\n".join(sorted_function_choices)
+    If a single runnable can't be determined, show CLI help indicating valid choices.
+    """
     filtered_local_entrypoints = [
-        name
-        for name, entrypoint in app.registered_entrypoints.items()
+        entrypoint
+        for entrypoint in app.registered_entrypoints.values()
         if entrypoint.info.module_name == module.__name__
     ]
 
-    if accept_local_entrypoint and len(filtered_local_entrypoints) == 1:
-        # If there is just a single local entrypoint in the target module, use
-        # that regardless of other functions.
-        function_name = list(filtered_local_entrypoints)[0]
-    elif accept_local_entrypoint and len(app.registered_entrypoints) == 1:
-        # Otherwise, if there is just a single local entrypoint in the stub as a whole,
-        # use that one.
-        function_name = list(app.registered_entrypoints.keys())[0]
-    elif len(function_choices) == 1:
-        function_name = sorted_function_choices[0]
-    elif len(function_choices) == 0:
+    if accept_local_entrypoint:
+        if len(filtered_local_entrypoints) == 1:
+            # If there is just a single local entrypoint in the target module, use
+            # that regardless of other functions.
+            return filtered_local_entrypoints[0]
+        elif len(app.registered_entrypoints) == 1:
+            # Otherwise, if there is just a single local entrypoint in the app as a whole,
+            # use that one.
+            return list(app.registered_entrypoints.values())[0]
+
+    # TODO: refactor registered_functions to only contain function services, not class services
+    function_choices: dict[str, Union[Function, LocalEntrypoint, MethodReference]] = {
+        name: f for name, f in app.registered_functions.items() if not name.endswith(".*")
+    }
+    for cls_name, cls in app.registered_classes.items():
+        for method_name in cls._get_method_names():
+            function_choices[f"{cls_name}.{method_name}"] = MethodReference(cls, method_name)
+
+    if not accept_webhook:
+        for web_endpoint_name in app.registered_web_endpoints:
+            function_choices.pop(web_endpoint_name, None)
+
+    if accept_local_entrypoint:
+        function_choices.update(app.registered_entrypoints)
+
+    if len(function_choices) == 1:
+        return list(function_choices.values())[0]
+
+    if len(function_choices) == 0:
         if app.registered_web_endpoints:
             err_msg = "Modal app has only web endpoints. Use `modal serve` instead of `modal run`."
         else:
             err_msg = "Modal app has no registered functions. Nothing to run."
         raise click.UsageError(err_msg)
-    else:
-        help_text = f"""You need to specify a Modal function or local entrypoint to run, e.g.
+
+    # there are multiple choices - we can't determine which one to use:
+    registered_functions_str = "\n".join(sorted(function_choices))
+    help_text = f"""You need to specify a Modal function or local entrypoint to run, e.g.
 
 modal run app.py::my_function [...args]
 
 Registered functions and local entrypoints on the selected app are:
 {registered_functions_str}
 """
-        raise click.UsageError(help_text)
-
-    if function_name in app.registered_entrypoints:
-        # entrypoint is in entrypoint registry, for now
-        return app.registered_entrypoints[function_name]
-
-    function = app.registered_functions[function_name]
-    assert isinstance(function, Function)
-    return function
+    raise click.UsageError(help_text)
 
 
 def _show_no_auto_detectable_app(app_ref: ImportRef) -> None:
@@ -223,30 +260,60 @@ def foo():
     error_console.print(guidance_msg)
 
 
-def import_function(
-    func_ref: str, base_cmd: str, accept_local_entrypoint=True, accept_webhook=False
-) -> Union[Function, LocalEntrypoint]:
+def _import_object(func_ref, base_cmd):
     import_ref = parse_import_ref(func_ref)
-
     module = import_file_or_module(import_ref.file_or_module)
-    app_or_function = get_by_object_path(module, import_ref.object_path or DEFAULT_APP_NAME)
+    app_function_or_method_ref = get_by_object_path(module, import_ref.object_path or DEFAULT_APP_NAME)
 
-    if app_or_function is None:
+    if app_function_or_method_ref is None:
         _show_function_ref_help(import_ref, base_cmd)
-        sys.exit(1)
+        raise SystemExit(1)
+
+    return app_function_or_method_ref, module
+
 
-    if isinstance(app_or_function, App):
+def _infer_runnable(
+    partial_obj: Union[App, Function, MethodReference, LocalEntrypoint],
+    module: types.ModuleType,
+    accept_local_entrypoint: bool = True,
+    accept_webhook: bool = False,
+) -> tuple[App, Union[Function, MethodReference, LocalEntrypoint]]:
+    if isinstance(partial_obj, App):
         # infer function or display help for how to select one
-        app = app_or_function
+        app = partial_obj
         function_handle = _infer_function_or_help(app, module, accept_local_entrypoint, accept_webhook)
-        return function_handle
-    elif isinstance(app_or_function, Function):
-        return app_or_function
-    elif isinstance(app_or_function, LocalEntrypoint):
+        return app, function_handle
+    elif isinstance(partial_obj, Function):
+        return partial_obj.app, partial_obj
+    elif isinstance(partial_obj, MethodReference):
+        return partial_obj.cls._get_app(), partial_obj
+    elif isinstance(partial_obj, LocalEntrypoint):
         if not accept_local_entrypoint:
             raise click.UsageError(
-                f"{func_ref} is not a Modal Function (a Modal local_entrypoint can't be used in this context)"
+                f"{partial_obj.info.function_name} is not a Modal Function "
+                f"(a Modal local_entrypoint can't be used in this context)"
             )
-        return app_or_function
+        return partial_obj.app, partial_obj
     else:
-        raise click.UsageError(f"{app_or_function} is not a Modal entity (should be an App or Function)")
+        raise click.UsageError(
+            f"{partial_obj} is not a Modal entity (should be an App, Local entrypoint, " "Function or Class/Method)"
+        )
+
+
+def import_and_infer(
+    func_ref: str, base_cmd: str, accept_local_entrypoint=True, accept_webhook=False
+) -> tuple[App, Union[Function, LocalEntrypoint, MethodReference]]:
+    """Takes a function ref string and returns something "runnable"
+
+    The function ref can leave out partial information (apart from the file name) as
+    long as the runnable is uniquely identifiable by the provided information.
+
+    When there are multiple runnables within the provided ref, the following rules should
+    be followed:
+
+    1. if there is a single local_entrypoint, that one is used
+    2. if there is a single {function, class} that one is used
+    3. if there is a single method (within a class) that one is used
+    """
+    app_function_or_method_ref, module = _import_object(func_ref, base_cmd)
+    return _infer_runnable(app_function_or_method_ref, module, accept_local_entrypoint, accept_webhook)

modal/cli/launch.py

@@ -8,11 +8,11 @@
 
 from typer import Typer
 
-from ..app import App
+from ..app import LocalEntrypoint
 from ..exception import _CliUserExecutionError
 from ..output import enable_output
 from ..runner import run_app
-from .import_refs import import_function
+from .import_refs import import_and_infer
 
 launch_cli = Typer(
     name="launch",
@@ -29,8 +29,10 @@ def _launch_program(name: str, filename: str, detach: bool, args: dict[str, Any]
     os.environ["MODAL_LAUNCH_ARGS"] = json.dumps(args)
 
     program_path = str(Path(__file__).parent / "programs" / filename)
-    entrypoint = import_function(program_path, "modal launch")
-    app: App = entrypoint.app
+    app, entrypoint = import_and_infer(program_path, "modal launch")
+    if not isinstance(entrypoint, LocalEntrypoint):
+        raise ValueError(f"{program_path} has no single local_entrypoint")
+
     app.set_description(f"modal launch {name}")
 
     # `launch/` scripts must have a `local_entrypoint()` with no args, for simplicity here.