Merge branch 'main' of github.com:modal-labs/modal-client into kramst…

…rom/cli-227-deprecate-mount-from-image-methods

CHANGELOG.md

@@ -10,6 +10,12 @@ We appreciate your patience while we speedily work towards a stable release of t
 
 <!-- NEW CONTENT GENERATED BELOW. PLEASE PRESERVE THIS COMMENT. -->
 
+### 0.72.8 (2025-01-10)
+
+- Fixes a bug introduced in v0.72.2 when specifying `add_python="3.9"` in `Image.from_registry`.
+
+
+
 ### 0.72.0 (2025-01-09)
 
 * The default behavior`Image.from_dockerfile()` and `image.dockerfile_commands()` if no parameter is passed to `ignore` will be to automatically detect if there is a valid dockerignore file in the current working directory or next to the dockerfile following the same rules as `dockerignore` does using `docker` commands. Previously no patterns were ignored.

modal/_utils/function_utils.py

@@ -326,11 +326,11 @@ def get_entrypoint_mount(self) -> list[_Mount]:
         # make sure the function's own entrypoint is included:
         if self._type == FunctionInfoType.PACKAGE:
             if config.get("automount"):
-                return [_Mount.from_local_python_packages(self.module_name)]
+                return [_Mount._from_local_python_packages(self.module_name)]
             elif not self.is_serialized():
                 # mount only relevant file and __init__.py:s
                 return [
-                    _Mount.from_local_dir(
+                    _Mount._from_local_dir(
                         self._base_dir,
                         remote_path=self._remote_dir,
                         recursive=True,
@@ -341,7 +341,7 @@ def get_entrypoint_mount(self) -> list[_Mount]:
             remote_path = ROOT_DIR / Path(self._file).name
             if not _is_modal_path(remote_path):
                 return [
-                    _Mount.from_local_file(
+                    _Mount._from_local_file(
                         self._file,
                         remote_path=remote_path,
                     )

modal/app.py

@@ -200,10 +200,9 @@ def __init__(
 
         ```python notest
         image = modal.Image.debian_slim().pip_install(...)
-        mount = modal.Mount.from_local_dir("./config")
         secret = modal.Secret.from_name("my-secret")
         volume = modal.Volume.from_name("my-data")
-        app = modal.App(image=image, mounts=[mount], secrets=[secret], volumes={"/mnt/data": volume})
+        app = modal.App(image=image, secrets=[secret], volumes={"/mnt/data": volume})
         ```
         """
         if name is not None and not isinstance(name, str):

modal/cli/launch.py

@@ -55,7 +55,7 @@ def jupyter(
     timeout: int = 3600,
     image: str = "ubuntu:22.04",
     add_python: Optional[str] = "3.11",
-    mount: Optional[str] = None,  # Create a `modal.Mount` from a local directory.
+    mount: Optional[str] = None,  # Adds a local directory to the jupyter container
     volume: Optional[str] = None,  # Attach a persisted `modal.Volume` by name (creating if missing).
     detach: bool = False,  # Run the app in "detached" mode to persist after local client disconnects
 ):

modal/cli/programs/run_jupyter.py

@@ -10,25 +10,20 @@
 import webbrowser
 from typing import Any
 
-from modal import App, Image, Mount, Queue, Secret, Volume, forward
+from modal import App, Image, Queue, Secret, Volume, forward
 
 # Passed by `modal launch` locally via CLI, plumbed to remote runner through secrets.
 args: dict[str, Any] = json.loads(os.environ.get("MODAL_LAUNCH_ARGS", "{}"))
 
-
 app = App()
-app.image = Image.from_registry(args.get("image"), add_python=args.get("add_python")).pip_install("jupyterlab")
 
+image = Image.from_registry(args.get("image"), add_python=args.get("add_python")).pip_install("jupyterlab")
 
-mount = (
-    Mount.from_local_dir(
+if args.get("mount"):
+    image = image.add_local_dir(
         args.get("mount"),
         remote_path="/root/lab/mount",
     )
-    if args.get("mount")
-    else None
-)
-mounts = [mount] if mount else []
 
 volume = (
     Volume.from_name(
@@ -55,12 +50,12 @@ def wait_for_port(url: str, q: Queue):
 
 
 @app.function(
+    image=image,
     cpu=args.get("cpu"),
     memory=args.get("memory"),
     gpu=args.get("gpu"),
     timeout=args.get("timeout"),
     secrets=[Secret.from_dict({"MODAL_LAUNCH_ARGS": json.dumps(args)})],
-    mounts=mounts,
     volumes=volumes,
     concurrency_limit=1 if volume else None,
 )

modal/cli/programs/vscode.py

@@ -10,7 +10,7 @@
 import webbrowser
 from typing import Any
 
-from modal import App, Image, Mount, Queue, Secret, Volume, forward
+from modal import App, Image, Queue, Secret, Volume, forward
 
 # Passed by `modal launch` locally via CLI, plumbed to remote runner through secrets.
 args: dict[str, Any] = json.loads(os.environ.get("MODAL_LAUNCH_ARGS", "{}"))
@@ -23,7 +23,7 @@
 
 
 app = App()
-app.image = (
+image = (
     Image.from_registry(args.get("image"), add_python="3.11")
     .apt_install("curl", "dumb-init", "git", "git-lfs")
     .run_commands(
@@ -44,16 +44,11 @@
     .env({"ENTRYPOINTD": ""})
 )
 
-
-mount = (
-    Mount.from_local_dir(
+if args.get("mount"):
+    image = image.add_local_dir(
         args.get("mount"),
         remote_path="/home/coder/mount",
     )
-    if args.get("mount")
-    else None
-)
-mounts = [mount] if mount else []
 
 volume = (
     Volume.from_name(
@@ -80,12 +75,12 @@ def wait_for_port(data: tuple[str, str], q: Queue):
 
 
 @app.function(
+    image=image,
     cpu=args.get("cpu"),
     memory=args.get("memory"),
     gpu=args.get("gpu"),
     timeout=args.get("timeout"),
     secrets=[Secret.from_dict({"MODAL_LAUNCH_ARGS": json.dumps(args)})],
-    mounts=mounts,
     volumes=volumes,
     concurrency_limit=1 if volume else None,
 )

modal/cls.py

@@ -16,7 +16,7 @@
 from ._serialization import check_valid_cls_constructor_arg
 from ._traceback import print_server_warnings
 from ._utils.async_utils import synchronize_api, synchronizer
-from ._utils.deprecation import renamed_parameter
+from ._utils.deprecation import deprecation_warning, renamed_parameter
 from ._utils.grpc_utils import retry_transient_errors
 from ._utils.mount_utils import validate_volumes
 from .client import _Client
@@ -374,12 +374,14 @@ class _Cls(_Object, type_prefix="cs"):
     _options: Optional[api_pb2.FunctionOptions]
     _callables: dict[str, Callable[..., Any]]
     _app: Optional["modal.app._App"] = None  # not set for lookups
+    _name: Optional[str]
 
     def _initialize_from_empty(self):
         self._user_cls = None
         self._class_service_function = None
         self._options = None
         self._callables = {}
+        self._name = None
 
     def _initialize_from_other(self, other: "_Cls"):
         super()._initialize_from_other(other)
@@ -388,6 +390,7 @@ def _initialize_from_other(self, other: "_Cls"):
         self._method_functions = other._method_functions
         self._options = other._options
         self._callables = other._callables
+        self._name = other._name
 
     def _get_partial_functions(self) -> dict[str, _PartialFunction]:
         if not self._user_cls:
@@ -506,6 +509,7 @@ async def _load(self: "_Cls", resolver: Resolver, existing_object_id: Optional[s
         cls._class_service_function = class_service_function
         cls._method_functions = method_functions
         cls._callables = callables
+        cls._name = user_cls.__name__
         return cls
 
     def _uses_common_service_function(self):
@@ -576,6 +580,7 @@ async def _load_remote(obj: _Object, resolver: Resolver, existing_object_id: Opt
         rep = f"Ref({app_name})"
         cls = cls._from_loader(_load_remote, rep, is_another_app=True, hydrate_lazily=True)
         # TODO: when pre 0.63 is phased out, we can set class_service_function here instead
+        cls._name = name
         return cls
 
     def with_options(
@@ -681,6 +686,13 @@ def __getattr__(self, k):
         # Used by CLI and container entrypoint
         # TODO: remove this method - access to attributes on classes should be discouraged
         if k in self._method_functions:
+            deprecation_warning(
+                (2025, 1, 13),
+                "Usage of methods directly on the class will soon be deprecated, "
+                "instantiate classes before using methods, e.g.:\n"
+                f"{self._name}().{k} instead of {self._name}.{k}",
+                pending=True,
+            )
             return self._method_functions[k]
         return getattr(self._user_cls, k)
 

modal/file_pattern_matcher.py

@@ -35,7 +35,7 @@ def __invert__(self) -> "_AbstractPatternMatcher":
         """
         return _CustomPatternMatcher(lambda path: not self(path))
 
-    def with_repr(self, custom_repr) -> "_AbstractPatternMatcher":
+    def _with_repr(self, custom_repr) -> "_AbstractPatternMatcher":
         # use to give an instance of a matcher a custom name - useful for visualizing default values in signatures
         self._custom_repr = custom_repr
         return self
@@ -60,7 +60,24 @@ def __call__(self, path: Path) -> bool:
 
 
 class FilePatternMatcher(_AbstractPatternMatcher):
-    """Allows matching file paths against a list of patterns."""
+    """
+    Allows matching file Path objects against a list of patterns.
+
+    **Usage:**
+    ```python
+    from pathlib import Path
+    from modal import FilePatternMatcher
+
+    matcher = FilePatternMatcher("*.py")
+
+    assert matcher(Path("foo.py"))
+
+    # You can also negate the matcher.
+    negated_matcher = ~matcher
+
+    assert not negated_matcher(Path("foo.py"))
+    ```
+    """
 
     patterns: list[Pattern]
     _delayed_init: Callable[[], None] = None
@@ -102,6 +119,15 @@ def from_file(cls, file_path: Path) -> "FilePatternMatcher":
 
         Args:
             file_path (Path): The path to the file containing patterns.
+
+        **Usage:**
+        ```python
+        from pathlib import Path
+        from modal import FilePatternMatcher
+
+        matcher = FilePatternMatcher.from_file(Path("/path/to/ignorefile"))
+        ```
+
         """
         uninitialized = cls.__new__(cls)
 
@@ -151,33 +177,15 @@ def _matches(self, file_path: str) -> bool:
         return matched
 
     def __call__(self, file_path: Path) -> bool:
-        """Check if the path matches any of the patterns.
-
-        Args:
-            file_path (Path): The path to check.
-
-        Returns:
-            True if the path matches any of the patterns.
-
-        Usage:
-        ```python
-        from pathlib import Path
-        from modal import FilePatternMatcher
-
-        matcher = FilePatternMatcher("*.py")
-
-        assert matcher(Path("foo.py"))
-        ```
-        """
         if self._delayed_init:
             self._delayed_init()
         return self._matches(str(file_path))
 
 
-# with_repr allows us to use this matcher as a default value in a function signature
+# _with_repr allows us to use this matcher as a default value in a function signature
 #  and get a nice repr in the docs and auto-generated type stubs:
-NON_PYTHON_FILES = (~FilePatternMatcher("**/*.py")).with_repr(f"{__name__}.NON_PYTHON_FILES")
-_NOTHING = (~FilePatternMatcher()).with_repr(f"{__name__}._NOTHING")  # match everything = ignore nothing
+NON_PYTHON_FILES = (~FilePatternMatcher("**/*.py"))._with_repr(f"{__name__}.NON_PYTHON_FILES")
+_NOTHING = (~FilePatternMatcher())._with_repr(f"{__name__}._NOTHING")  # match everything = ignore nothing
 
 
 def _ignore_fn(ignore: Union[Sequence[str], Callable[[Path], bool]]) -> Callable[[Path], bool]:

modal/image.py

@@ -715,7 +715,7 @@ def add_local_file(self, local_path: Union[str, Path], remote_path: str, *, copy
         if remote_path.endswith("/"):
             remote_path = remote_path + Path(local_path).name
 
-        mount = _Mount.from_local_file(local_path, remote_path)
+        mount = _Mount._from_local_file(local_path, remote_path)
         return self._add_mount_layer_or_copy(mount, copy=copy)
 
     def add_local_dir(
@@ -803,7 +803,7 @@ def build_dockerfile(version: ImageBuilderVersion) -> DockerfileSpec:
         return _Image._from_args(
             base_images={"base": self},
             dockerfile_function=build_dockerfile,
-            context_mount_function=lambda: _Mount.from_local_file(local_path, remote_path=f"/{basename}"),
+            context_mount_function=lambda: _Mount._from_local_file(local_path, remote_path=f"/{basename}"),
         )
 
     def add_local_python_source(
@@ -841,7 +841,7 @@ def add_local_python_source(
         )
         ```
         """
-        mount = _Mount.from_local_python_packages(*modules, ignore=ignore)
+        mount = _Mount._from_local_python_packages(*modules, ignore=ignore)
         return self._add_mount_layer_or_copy(mount, copy=copy)
 
     def copy_local_dir(
@@ -1522,7 +1522,8 @@ def _registry_setup_commands(
                 "COPY /python/. /usr/local",
                 "ENV TERMINFO_DIRS=/etc/terminfo:/lib/terminfo:/usr/share/terminfo:/usr/lib/terminfo",
             ]
-            if add_python < "3.13":
+            python_minor = add_python.split(".")[1]
+            if int(python_minor) < 13:
                 # Previous versions did not include the `python` binary, but later ones do.
                 # (The important factor is not the Python version itself, but the standalone dist version.)
                 # We insert the command in the list at the position it was previously always added