From 8bc041c9b73b70e17e42343eff9113c7b14b9a0b Mon Sep 17 00:00:00 2001 From: Elias Freider <elias@modal.com> Date: Mon, 2 Sep 2024 15:58:41 +0200 Subject: [PATCH 01/36] wip --- modal/image.py | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/modal/image.py b/modal/image.py index bdd0dfa8a..b12e1ef0e 100644 --- a/modal/image.py +++ b/modal/image.py @@ -264,9 +264,11 @@ class _Image(_Object, type_prefix="im"): force_build: bool inside_exceptions: List[Exception] + _mounts: Sequence[_Mount] def _initialize_from_empty(self): self.inside_exceptions = [] + self._mounts = () def _hydrate_metadata(self, message: Optional[Message]): env_image_id = config.get("image_id") @@ -288,6 +290,7 @@ def _from_args( force_build: bool = False, # For internal use only. _namespace: int = api_pb2.DEPLOYMENT_NAMESPACE_WORKSPACE, + _mounts: Sequence[_Mount] = (), ): if base_images is None: base_images = {} @@ -523,6 +526,23 @@ def build_dockerfile(version: ImageBuilderVersion) -> DockerfileSpec: context_mount=mount, ) + def copy_local_file2( + self, local_path: Union[str, Path], remote_path: Optional[Union[str, Path]] = None + ) -> "_Image": + """Copy a file into the image as a part of building it. + + This works in a similar way to [`COPY`](https://docs.docker.com/engine/reference/builder/#copy) + works in a `Dockerfile`. + """ + if remote_path is None: + # TODO: track workdir (!) to put file in workdir/basename, OR let mounts support relative paths + basename = str(Path(local_path).name) + remote_path = Path("/root") / basename + + mount = _Mount.from_local_file(local_path, remote_path=remote_path) + + return _Image._from_args(base_images={"base": self}, _mounts=self._mounts + (mount,)) + def copy_local_dir(self, local_path: Union[str, Path], remote_path: Union[str, Path] = ".") -> "_Image": """Copy a directory into the image as a part of building the image. @@ -1711,5 +1731,9 @@ async def _logs(self) -> AsyncGenerator[str, None]: if task_log.data: yield task_log.data + def _split_mounts(self) -> Tuple["_Image", Sequence[_Mount]]: + if self._mounts: + assert not self.dockerfile_commands + Image = synchronize_api(_Image) From b8d399f2636bf8f02ba0dc7c53b45dc67abb0c72 Mon Sep 17 00:00:00 2001 From: Elias Freider <elias@modal.com> Date: Mon, 23 Sep 2024 16:42:25 +0200 Subject: [PATCH 02/36] wip --- modal/image.py | 19 ++++++------------- test/image_test.py | 22 ++++++++++++++++++++++ 2 files changed, 28 insertions(+), 13 deletions(-) diff --git a/modal/image.py b/modal/image.py index b12e1ef0e..0d7a5afc4 100644 --- a/modal/image.py +++ b/modal/image.py @@ -463,6 +463,7 @@ async def join(): rep = "Image()" obj = _Image._from_loader(_load, rep, deps=_deps) obj.force_build = force_build + obj._mounts = _mounts return obj def extend(self, **kwargs) -> "_Image": @@ -526,21 +527,13 @@ def build_dockerfile(version: ImageBuilderVersion) -> DockerfileSpec: context_mount=mount, ) - def copy_local_file2( - self, local_path: Union[str, Path], remote_path: Optional[Union[str, Path]] = None - ) -> "_Image": - """Copy a file into the image as a part of building it. + def add_local_python_packages(self, *packages: Union[str, Path]) -> "_Image": + """Adds local Python packages to the image - This works in a similar way to [`COPY`](https://docs.docker.com/engine/reference/builder/#copy) - works in a `Dockerfile`. + Packages are added to the /root directory which is on the PYTHONPATH of any + executed Modal functions. """ - if remote_path is None: - # TODO: track workdir (!) to put file in workdir/basename, OR let mounts support relative paths - basename = str(Path(local_path).name) - remote_path = Path("/root") / basename - - mount = _Mount.from_local_file(local_path, remote_path=remote_path) - + mount = _Mount.from_local_python_packages(*packages) return _Image._from_args(base_images={"base": self}, _mounts=self._mounts + (mount,)) def copy_local_dir(self, local_path: Union[str, Path], remote_path: Union[str, Path] = ".") -> "_Image": diff --git a/test/image_test.py b/test/image_test.py index 5da46bce3..cf01d6779 100644 --- a/test/image_test.py +++ b/test/image_test.py @@ -12,6 +12,7 @@ from modal import App, Image, Mount, Secret, build, gpu, method from modal._serialization import serialize +from modal._utils.async_utils import synchronizer from modal.client import Client from modal.exception import DeprecationError, InvalidError, VersionError from modal.image import ( @@ -1154,3 +1155,24 @@ async def test_logs(servicer, client): logs = [data async for data in image._logs.aio()] assert logs == ["build starting\n", "build finished\n"] + + +def test_add_local_python_packages(): + image = Image.debian_slim().add_local_python_packages("pkg_a") + + _image = synchronizer._translate_in(image) + mount_layer = _image._mounts + pkg_mount: Mount = mount_layer[0] + from modal.mount import _MountedPythonModule + + assert len(pkg_mount.entries) == 1 + mount_entry = pkg_mount.entries[0] + assert isinstance(mount_entry, _MountedPythonModule) + assert mount_entry.remote_dir == "/root" + assert mount_entry.module_name == "pkg_a" + + image_additional_mount = image.add_local_python_packages("pkg_b") + assert len(synchronizer._translate_in(image_additional_mount)._mounts) == 2 + + image_non_mount = image.run_commands("echo 'hello'") + assert synchronizer._translate_in(image_non_mount)._mounts == () # mounts don't transfer for non-mount operations From 5e34e80be5ef88d92c5adaadb76e890b4f3738e1 Mon Sep 17 00:00:00 2001 From: Elias Freider <elias@modal.com> Date: Thu, 26 Sep 2024 13:01:00 +0200 Subject: [PATCH 03/36] wip --- modal/image.py | 27 +++++++++++++++++++-------- test/image_test.py | 17 +++++++++++++++-- 2 files changed, 34 insertions(+), 10 deletions(-) diff --git a/modal/image.py b/modal/image.py index 0d7a5afc4..48fa6df51 100644 --- a/modal/image.py +++ b/modal/image.py @@ -290,7 +290,7 @@ def _from_args( force_build: bool = False, # For internal use only. _namespace: int = api_pb2.DEPLOYMENT_NAMESPACE_WORKSPACE, - _mounts: Sequence[_Mount] = (), + _mounts: Sequence[_Mount] = (), # used for "soft/lazy" layers ): if base_images is None: base_images = {} @@ -326,12 +326,25 @@ async def _load(self: _Image, resolver: Resolver, existing_object_id: Optional[s else: dockerfile = dockerfile_function(builder_version) - if not dockerfile.commands and not build_function: + if not dockerfile.commands and not build_function and not _mounts: raise InvalidError( "No commands were provided for the image — have you tried using modal.Image.debian_slim()?" ) - if dockerfile.commands and build_function: - raise InvalidError("Cannot provide both a build function and Dockerfile commands!") + if bool(dockerfile.commands) + bool(build_function) + bool(_mounts) > 1: + raise InvalidError( + "Cannot provide multiple of: build function, Dockerfile commands and lazy mounts in the same image layer!" + ) + + if _mounts: + if len(base_images) > 1: + raise InvalidError("Mount can't be used directly on multiple base images") + + # for building purposes, this image should be the same as the base image, until + # another non-mount layer is added on top of it + base_image = list(base_images.values())[0] + soft_layer_image = base_image.clone() + soft_layer_image._mounts = _mounts + return soft_layer_image base_images_pb2s = [ api_pb2.BaseImage( @@ -463,7 +476,6 @@ async def join(): rep = "Image()" obj = _Image._from_loader(_load, rep, deps=_deps) obj.force_build = force_build - obj._mounts = _mounts return obj def extend(self, **kwargs) -> "_Image": @@ -1724,9 +1736,8 @@ async def _logs(self) -> AsyncGenerator[str, None]: if task_log.data: yield task_log.data - def _split_mounts(self) -> Tuple["_Image", Sequence[_Mount]]: - if self._mounts: - assert not self.dockerfile_commands + def _layers(self) -> typing.Tuple["_Image"]: + pass Image = synchronize_api(_Image) diff --git a/test/image_test.py b/test/image_test.py index cf01d6779..4007beb21 100644 --- a/test/image_test.py +++ b/test/image_test.py @@ -1157,8 +1157,9 @@ async def test_logs(servicer, client): assert logs == ["build starting\n", "build finished\n"] -def test_add_local_python_packages(): - image = Image.debian_slim().add_local_python_packages("pkg_a") +def test_add_local_python_packages(client, servicer): + deb = Image.debian_slim() + image = deb.add_local_python_packages("pkg_a") _image = synchronizer._translate_in(image) mount_layer = _image._mounts @@ -1176,3 +1177,15 @@ def test_add_local_python_packages(): image_non_mount = image.run_commands("echo 'hello'") assert synchronizer._translate_in(image_non_mount)._mounts == () # mounts don't transfer for non-mount operations + + # make sure that the run_commands instroduce a new layer that copies the mount + app = App() + + app.function(serialized=True, image=image)(lambda: None) + with app.run(client=client): + layers = get_image_layers(image.object_id, servicer) + for i, layer in enumerate(layers): + print("Layer", i) + print(layer) + commands = [layer.dockerfile_commands for layer in layers] + context_files = [[(f.filename, f.data) for f in layer.context_files] for layer in layers] From af9f74f2dfeb57abe4f46a40f9ff64223bc364ff Mon Sep 17 00:00:00 2001 From: Elias Freider <elias@modal.com> Date: Mon, 30 Sep 2024 09:57:55 +0200 Subject: [PATCH 04/36] wip --- modal/image.py | 12 +++++++++--- test/image_test.py | 20 ++++++++++---------- 2 files changed, 19 insertions(+), 13 deletions(-) diff --git a/modal/image.py b/modal/image.py index 48fa6df51..79158dfa1 100644 --- a/modal/image.py +++ b/modal/image.py @@ -264,11 +264,16 @@ class _Image(_Object, type_prefix="im"): force_build: bool inside_exceptions: List[Exception] - _mounts: Sequence[_Mount] + _mounts: Optional[Sequence[_Mount]] def _initialize_from_empty(self): self.inside_exceptions = [] - self._mounts = () + self._mounts = None + + def _initialize_from_other(self, other): + # used by .clone() + self.inside_exceptions = other.inside_exceptions + self._mounts = other._mounts.copy() if other._mounts is not None else None def _hydrate_metadata(self, message: Optional[Message]): env_image_id = config.get("image_id") @@ -339,11 +344,12 @@ async def _load(self: _Image, resolver: Resolver, existing_object_id: Optional[s if len(base_images) > 1: raise InvalidError("Mount can't be used directly on multiple base images") + print("MOUNT LAYER", _mounts) # for building purposes, this image should be the same as the base image, until # another non-mount layer is added on top of it base_image = list(base_images.values())[0] soft_layer_image = base_image.clone() - soft_layer_image._mounts = _mounts + soft_layer_image._mounts.append(_mounts) return soft_layer_image base_images_pb2s = [ diff --git a/test/image_test.py b/test/image_test.py index 4007beb21..426e36cb7 100644 --- a/test/image_test.py +++ b/test/image_test.py @@ -1157,20 +1157,20 @@ async def test_logs(servicer, client): assert logs == ["build starting\n", "build finished\n"] -def test_add_local_python_packages(client, servicer): +def test_add_local_python_packages(client, servicer, set_env_client): deb = Image.debian_slim() image = deb.add_local_python_packages("pkg_a") - _image = synchronizer._translate_in(image) - mount_layer = _image._mounts - pkg_mount: Mount = mount_layer[0] - from modal.mount import _MountedPythonModule + # _image = synchronizer._translate_in(image) + # mount_layer = _image._mounts + # pkg_mount: Mount = mount_layer[0] + # from modal.mount import _MountedPythonModule - assert len(pkg_mount.entries) == 1 - mount_entry = pkg_mount.entries[0] - assert isinstance(mount_entry, _MountedPythonModule) - assert mount_entry.remote_dir == "/root" - assert mount_entry.module_name == "pkg_a" + # assert len(pkg_mount.entries) == 1 + # mount_entry = pkg_mount.entries[0] + # assert isinstance(mount_entry, _MountedPythonModule) + # assert mount_entry.remote_dir == "/root" + # assert mount_entry.module_name == "pkg_a" image_additional_mount = image.add_local_python_packages("pkg_b") assert len(synchronizer._translate_in(image_additional_mount)._mounts) == 2 From 2a52f6d721e0e84617a61002ee4a4583295d5c5d Mon Sep 17 00:00:00 2001 From: Elias Freider <elias@modal.com> Date: Wed, 16 Oct 2024 10:55:31 +0200 Subject: [PATCH 05/36] Working consolidation of mounts --- modal/image.py | 79 ++++++++++++++++++++++++++++++++++------------ test/image_test.py | 57 ++++++++++++++++++--------------- test/mount_test.py | 9 +++--- 3 files changed, 94 insertions(+), 51 deletions(-) diff --git a/modal/image.py b/modal/image.py index 41c59dc9d..f20ba1e21 100644 --- a/modal/image.py +++ b/modal/image.py @@ -274,16 +274,18 @@ class _Image(_Object, type_prefix="im"): force_build: bool inside_exceptions: List[Exception] - _mounts: Optional[Sequence[_Mount]] + _mounts: Tuple[_Mount] def _initialize_from_empty(self): self.inside_exceptions = [] - self._mounts = None + self._mounts = () + self.force_build = False def _initialize_from_other(self, other): # used by .clone() self.inside_exceptions = other.inside_exceptions - self._mounts = other._mounts.copy() if other._mounts is not None else None + self.force_build = other.force_build + self._mounts = other._mounts def _hydrate_metadata(self, message: Optional[Message]): env_image_id = config.get("image_id") @@ -291,6 +293,39 @@ def _hydrate_metadata(self, message: Optional[Message]): for exc in self.inside_exceptions: raise exc + def _add_mount_layer(self, mounts: Sequence[_Mount] = ()): + base_image = self + + async def _load(self: _Image, resolver: Resolver, existing_object_id: Optional[str]): + self._hydrate_from_other(base_image) # same image id as base image as long as it's lazy + self._mounts = base_image._mounts + tuple(mounts) + + return _Image._from_loader(_load, "ImageWithMounts()", deps=lambda: [base_image] + list(mounts)) + + @property + def _stacked_mounts(self) -> typing.Tuple[_Mount]: + """Non-evaluated mount layers on the image + + When the image is used by a Modal container, these mounts need to be attached as well to + represent the full image content, as they haven't yet been represented as a layer in the + image. + + When the image is used as a base image for a new layer (that is not itself a mount layer) + these mounts need to first be inserted as a copy operation (.copy_mount) into the image. + """ + return self._mounts + + def _consolidate_mounts(self) -> "_Image": + """Takes any stacked mounts on the image and makes them into actual image layers using COPY + + This needs to be run before an image with mounts is used as a base image for anything + other than another "mount only" layer, e.g. a docker command. + """ + image = self + for mount in self._stacked_mounts: + image = image._consolidate_mount(mount) + return image + @staticmethod def _from_args( *, @@ -305,10 +340,12 @@ def _from_args( force_build: bool = False, # For internal use only. _namespace: int = api_pb2.DEPLOYMENT_NAMESPACE_WORKSPACE, - _mounts: Sequence[_Mount] = (), # used for "soft/lazy" layers + _consolidate_mounts: bool = True, ): if base_images is None: base_images = {} + else: + base_images = {k: v._consolidate_mounts() if _consolidate_mounts else v for k, v in base_images.items()} if secrets is None: secrets = [] if gpu_config is None: @@ -344,27 +381,15 @@ async def _load(self: _Image, resolver: Resolver, existing_object_id: Optional[s else: dockerfile = dockerfile_function(builder_version) - if not dockerfile.commands and not build_function and not _mounts: + if not dockerfile.commands and not build_function: raise InvalidError( "No commands were provided for the image — have you tried using modal.Image.debian_slim()?" ) - if bool(dockerfile.commands) + bool(build_function) + bool(_mounts) > 1: + if dockerfile.commands and build_function: raise InvalidError( - "Cannot provide multiple of: build function, Dockerfile commands and lazy mounts in the same image layer!" + "Cannot provide both build function and Dockerfile commands in the same image layer!" ) - if _mounts: - if len(base_images) > 1: - raise InvalidError("Mount can't be used directly on multiple base images") - - print("MOUNT LAYER", _mounts) - # for building purposes, this image should be the same as the base image, until - # another non-mount layer is added on top of it - base_image = list(base_images.values())[0] - soft_layer_image = base_image.clone() - soft_layer_image._mounts.append(_mounts) - return soft_layer_image - base_images_pb2s = [ api_pb2.BaseImage( docker_tag=docker_tag, @@ -492,7 +517,7 @@ async def join(): self._hydrate(image_id, resolver.client, None) - rep = "Image()" + rep = f"Image({dockerfile_function})" obj = _Image._from_loader(_load, rep, deps=_deps) obj.force_build = force_build return obj @@ -512,6 +537,18 @@ def build_dockerfile(version: ImageBuilderVersion) -> DockerfileSpec: return _Image._from_args(base_images={"base": self}, dockerfile_function=build_dockerfile, **kwargs) + def _consolidate_mount(self, mount: _Mount) -> "_Image": + def build_dockerfile(version: ImageBuilderVersion) -> DockerfileSpec: + commands = ["FROM base", "COPY . /"] # copy everything from the supplied mount into the root + return DockerfileSpec(commands=commands, context_files={}) + + return _Image._from_args( + base_images={"base": self}, + dockerfile_function=build_dockerfile, + context_mount=mount, + _consolidate_mounts=False, # avoid recursion + ) + def copy_mount(self, mount: _Mount, remote_path: Union[str, Path] = ".") -> "_Image": """Copy the entire contents of a `modal.Mount` into an image. Useful when files only available locally are required during the image @@ -565,7 +602,7 @@ def add_local_python_packages(self, *packages: Union[str, Path]) -> "_Image": executed Modal functions. """ mount = _Mount.from_local_python_packages(*packages) - return _Image._from_args(base_images={"base": self}, _mounts=self._mounts + (mount,)) + return self._add_mount_layer([mount]) def copy_local_dir(self, local_path: Union[str, Path], remote_path: Union[str, Path] = ".") -> "_Image": """Copy a directory into the image as a part of building the image. diff --git a/test/image_test.py b/test/image_test.py index 5a8b46523..ea7f01ca4 100644 --- a/test/image_test.py +++ b/test/image_test.py @@ -12,7 +12,6 @@ from modal import App, Image, Mount, Secret, build, environments, gpu, method from modal._serialization import serialize -from modal._utils.async_utils import synchronizer from modal.client import Client from modal.exception import DeprecationError, InvalidError, VersionError from modal.image import ( @@ -1099,35 +1098,43 @@ async def test_logs(servicer, client): assert logs == ["build starting\n", "build finished\n"] -def test_add_local_python_packages(client, servicer, set_env_client): +def test_add_local_python_packages(client, servicer, set_env_client, test_dir, monkeypatch): + monkeypatch.syspath_prepend((test_dir / "supports").as_posix()) deb = Image.debian_slim() image = deb.add_local_python_packages("pkg_a") - # _image = synchronizer._translate_in(image) - # mount_layer = _image._mounts - # pkg_mount: Mount = mount_layer[0] - # from modal.mount import _MountedPythonModule + def hydrate_image(img): + # there should be a more straight forward way to do this? + app = App() + app.function(serialized=True, image=img)(lambda: None) + with app.run(client=client): + pass + assert len(image._stacked_mounts) == 1 - # assert len(pkg_mount.entries) == 1 - # mount_entry = pkg_mount.entries[0] - # assert isinstance(mount_entry, _MountedPythonModule) - # assert mount_entry.remote_dir == "/root" - # assert mount_entry.module_name == "pkg_a" + hydrate_image(image) + assert len(image._stacked_mounts) == 1 image_additional_mount = image.add_local_python_packages("pkg_b") - assert len(synchronizer._translate_in(image_additional_mount)._mounts) == 2 + hydrate_image(image_additional_mount) + assert len(image_additional_mount._stacked_mounts) == 2 # another mount added to lazy layer + assert len(image._stacked_mounts) == 1 # original image should not be affected image_non_mount = image.run_commands("echo 'hello'") - assert synchronizer._translate_in(image_non_mount)._mounts == () # mounts don't transfer for non-mount operations - - # make sure that the run_commands instroduce a new layer that copies the mount - app = App() - - app.function(serialized=True, image=image)(lambda: None) - with app.run(client=client): - layers = get_image_layers(image.object_id, servicer) - for i, layer in enumerate(layers): - print("Layer", i) - print(layer) - commands = [layer.dockerfile_commands for layer in layers] - context_files = [[(f.filename, f.data) for f in layer.context_files] for layer in layers] + hydrate_image(image_non_mount) + assert len(image_non_mount._stacked_mounts) == 0 + # TODO: assert layers include copy of all mounts + echo + + layers = get_image_layers(image_non_mount.object_id, servicer) + for layer in layers: + print("===========LAYER========") + print(layer) + + echo_layer = layers[0] + assert echo_layer.dockerfile_commands == ["FROM base", "RUN echo 'hello'"] + + copy_layer = layers[1] + assert copy_layer.dockerfile_commands == ["FROM base", "COPY . /"] + assert copy_layer.context_mount_id + copied_files = servicer.mount_contents[copy_layer.context_mount_id].keys() + assert len(copied_files) == 8 + assert all(fn.startswith("/root/pkg_a/") for fn in copied_files) diff --git a/test/mount_test.py b/test/mount_test.py index f7aa24db0..4b342e9fa 100644 --- a/test/mount_test.py +++ b/test/mount_test.py @@ -3,7 +3,6 @@ import os import platform import pytest -import sys from pathlib import Path from modal import App @@ -88,10 +87,10 @@ def dummy(): pass -def test_from_local_python_packages(servicer, client, test_dir): +def test_from_local_python_packages(servicer, client, test_dir, monkeypatch): app = App() - sys.path.append((test_dir / "supports").as_posix()) + monkeypatch.syspath_prepend((test_dir / "supports").as_posix()) app.function(mounts=[Mount.from_local_python_packages("pkg_a", "pkg_b", "standalone_file")])(dummy) @@ -110,8 +109,8 @@ def test_from_local_python_packages(servicer, client, test_dir): assert "/root/pkg_c/j/k.py" not in files -def test_app_mounts(servicer, client, test_dir): - sys.path.append((test_dir / "supports").as_posix()) +def test_app_mounts(servicer, client, test_dir, monkeypatch): + monkeypatch.syspath_prepend((test_dir / "supports").as_posix()) app = App(mounts=[Mount.from_local_python_packages("pkg_b")]) From e23763439c6c208505ab65aef387203aebbc1086 Mon Sep 17 00:00:00 2001 From: Elias Freider <elias@modal.com> Date: Wed, 16 Oct 2024 13:44:33 +0200 Subject: [PATCH 06/36] Functions now attach lazy mounts to the container --- modal/functions.py | 4 +++- modal/image.py | 12 +++++----- test/image_test.py | 57 +++++++++++++++++++++++++++++++++++++++++++++- 3 files changed, 65 insertions(+), 8 deletions(-) diff --git a/modal/functions.py b/modal/functions.py index 9acf0ea7a..89eca02d8 100644 --- a/modal/functions.py +++ b/modal/functions.py @@ -550,6 +550,7 @@ def from_args( if is_local(): entrypoint_mounts = info.get_entrypoint_mount() + all_mounts = [ _get_client_mount(), *explicit_mounts, @@ -582,6 +583,7 @@ def from_args( if proxy: # HACK: remove this once we stop using ssh tunnels for this. if image: + # TODO(elias): this breaks lazy mounts by materializing them, which isn't great image = image.apt_install("autossh") function_spec = _FunctionSpec( @@ -786,7 +788,7 @@ async def _load(self: _Function, resolver: Resolver, existing_object_id: Optiona ) for path, volume in validated_volumes ] - loaded_mount_ids = {m.object_id for m in all_mounts} + loaded_mount_ids = {m.object_id for m in all_mounts} | {m.object_id for m in image._stacked_mounts} # Get object dependencies object_dependencies = [] diff --git a/modal/image.py b/modal/image.py index f20ba1e21..0e9535ae7 100644 --- a/modal/image.py +++ b/modal/image.py @@ -315,7 +315,7 @@ def _stacked_mounts(self) -> typing.Tuple[_Mount]: """ return self._mounts - def _consolidate_mounts(self) -> "_Image": + def _materialize_mounts(self) -> "_Image": """Takes any stacked mounts on the image and makes them into actual image layers using COPY This needs to be run before an image with mounts is used as a base image for anything @@ -323,7 +323,7 @@ def _consolidate_mounts(self) -> "_Image": """ image = self for mount in self._stacked_mounts: - image = image._consolidate_mount(mount) + image = image._materialize_mount(mount) return image @staticmethod @@ -340,12 +340,12 @@ def _from_args( force_build: bool = False, # For internal use only. _namespace: int = api_pb2.DEPLOYMENT_NAMESPACE_WORKSPACE, - _consolidate_mounts: bool = True, + _materialize_mounts: bool = True, ): if base_images is None: base_images = {} else: - base_images = {k: v._consolidate_mounts() if _consolidate_mounts else v for k, v in base_images.items()} + base_images = {k: v._materialize_mounts() if _materialize_mounts else v for k, v in base_images.items()} if secrets is None: secrets = [] if gpu_config is None: @@ -537,7 +537,7 @@ def build_dockerfile(version: ImageBuilderVersion) -> DockerfileSpec: return _Image._from_args(base_images={"base": self}, dockerfile_function=build_dockerfile, **kwargs) - def _consolidate_mount(self, mount: _Mount) -> "_Image": + def _materialize_mount(self, mount: _Mount) -> "_Image": def build_dockerfile(version: ImageBuilderVersion) -> DockerfileSpec: commands = ["FROM base", "COPY . /"] # copy everything from the supplied mount into the root return DockerfileSpec(commands=commands, context_files={}) @@ -546,7 +546,7 @@ def build_dockerfile(version: ImageBuilderVersion) -> DockerfileSpec: base_images={"base": self}, dockerfile_function=build_dockerfile, context_mount=mount, - _consolidate_mounts=False, # avoid recursion + _materialize_mounts=False, # avoid recursion ) def copy_mount(self, mount: _Mount, remote_path: Union[str, Path] = ".") -> "_Image": diff --git a/test/image_test.py b/test/image_test.py index ea7f01ca4..ed241962c 100644 --- a/test/image_test.py +++ b/test/image_test.py @@ -10,6 +10,7 @@ from typing import List, Literal, get_args from unittest import mock +import modal from modal import App, Image, Mount, Secret, build, environments, gpu, method from modal._serialization import serialize from modal.client import Client @@ -23,6 +24,7 @@ _validate_python_version, ) from modal.mount import PYTHON_STANDALONE_VERSIONS +from modal.runner import deploy_app from modal_proto import api_pb2 from .supports.skip import skip_windows @@ -1134,7 +1136,60 @@ def hydrate_image(img): copy_layer = layers[1] assert copy_layer.dockerfile_commands == ["FROM base", "COPY . /"] - assert copy_layer.context_mount_id + assert copy_layer.context_mount_id == image._stacked_mounts[0].object_id copied_files = servicer.mount_contents[copy_layer.context_mount_id].keys() assert len(copied_files) == 8 assert all(fn.startswith("/root/pkg_a/") for fn in copied_files) + + +def test_lazy_mounts_are_attached_to_functions(servicer, client, test_dir, monkeypatch): + monkeypatch.syspath_prepend((test_dir / "supports").as_posix()) + deb_slim = Image.debian_slim() + img = deb_slim.add_local_python_packages("pkg_a") + app = App("my-app") + control_fun = app.function(serialized=True, image=deb_slim, name="control")(lambda: None) # no mounts on image + fun = app.function(serialized=True, image=img, name="fun")(lambda: None) # mounts on image + deploy_app(app, client=client) + + control_func_mounts = set(servicer.app_functions[control_fun.object_id].mount_ids) + fun_def = servicer.app_functions[fun.object_id] + added_mounts = set(fun_def.mount_ids) - control_func_mounts + assert len(added_mounts) == 1 + assert added_mounts == {img._stacked_mounts[0].object_id} + + +def test_lazy_mounts_are_attached_to_classes(servicer, client, test_dir, monkeypatch): + monkeypatch.syspath_prepend((test_dir / "supports").as_posix()) + deb_slim = Image.debian_slim() + img = deb_slim.add_local_python_packages("pkg_a") + app = App("my-app") + control_fun = app.function(serialized=True, image=deb_slim, name="control")(lambda: None) # no mounts on image + + class A: + some_arg: str = modal.parameter() + + ACls = app.cls(serialized=True, image=img)(A) # mounts on image + deploy_app(app, client=client) + + control_func_mounts = set(servicer.app_functions[control_fun.object_id].mount_ids) + fun_def = servicer.function_by_name("A.*") # class service function + added_mounts = set(fun_def.mount_ids) - control_func_mounts + assert len(added_mounts) == 1 + assert added_mounts == {img._stacked_mounts[0].object_id} + + obj = ACls(some_arg="foo") + obj.keep_warm(0) # hacky way to force hydration of the *parameter bound* function (instance service function) + obj_fun_def = servicer.function_by_name("A.*", ((), {"some_arg": "foo"})) # instance service function + added_mounts = set(obj_fun_def.mount_ids) - control_func_mounts + assert len(added_mounts) == 1 + assert added_mounts == {img._stacked_mounts[0].object_id} + + +# def test_lazy_mounts_are_attached_to_sandboxes(servicer, client, test_dir, monkeypatch): +# monkeypatch.syspath_prepend((test_dir / "supports").as_posix()) +# img = Image.debian_slim().add_local_python_packages("pkg_a") + + +# TODO: test build functions w/ lazy mounts + +# TODO: test modal serve w/ lazy mounts From 787dcc6942266224dad8b584ca15c173474aab5d Mon Sep 17 00:00:00 2001 From: Elias Freider <elias@modal.com> Date: Wed, 16 Oct 2024 13:58:25 +0200 Subject: [PATCH 07/36] Fix sandboxes --- modal/functions.py | 2 +- modal/image.py | 4 ++-- modal/sandbox.py | 2 +- test/image_test.py | 32 ++++++++++++++++++++------------ 4 files changed, 24 insertions(+), 16 deletions(-) diff --git a/modal/functions.py b/modal/functions.py index 89eca02d8..ebafe2ddf 100644 --- a/modal/functions.py +++ b/modal/functions.py @@ -788,7 +788,7 @@ async def _load(self: _Function, resolver: Resolver, existing_object_id: Optiona ) for path, volume in validated_volumes ] - loaded_mount_ids = {m.object_id for m in all_mounts} | {m.object_id for m in image._stacked_mounts} + loaded_mount_ids = {m.object_id for m in all_mounts} | {m.object_id for m in image._mount_layers} # Get object dependencies object_dependencies = [] diff --git a/modal/image.py b/modal/image.py index 0e9535ae7..78517a2c3 100644 --- a/modal/image.py +++ b/modal/image.py @@ -303,7 +303,7 @@ async def _load(self: _Image, resolver: Resolver, existing_object_id: Optional[s return _Image._from_loader(_load, "ImageWithMounts()", deps=lambda: [base_image] + list(mounts)) @property - def _stacked_mounts(self) -> typing.Tuple[_Mount]: + def _mount_layers(self) -> typing.Tuple[_Mount]: """Non-evaluated mount layers on the image When the image is used by a Modal container, these mounts need to be attached as well to @@ -322,7 +322,7 @@ def _materialize_mounts(self) -> "_Image": other than another "mount only" layer, e.g. a docker command. """ image = self - for mount in self._stacked_mounts: + for mount in self._mount_layers: image = image._materialize_mount(mount) return image diff --git a/modal/sandbox.py b/modal/sandbox.py index f2e11f9bd..8494725b8 100644 --- a/modal/sandbox.py +++ b/modal/sandbox.py @@ -144,7 +144,7 @@ async def _load(self: _Sandbox, resolver: Resolver, _existing_object_id: Optiona definition = api_pb2.Sandbox( entrypoint_args=entrypoint_args, image_id=image.object_id, - mount_ids=[mount.object_id for mount in mounts], + mount_ids=[mount.object_id for mount in mounts] + [mount.object_id for mount in image._mount_layers], secret_ids=[secret.object_id for secret in secrets], timeout_secs=timeout, workdir=workdir, diff --git a/test/image_test.py b/test/image_test.py index ed241962c..8ee631e12 100644 --- a/test/image_test.py +++ b/test/image_test.py @@ -1111,19 +1111,19 @@ def hydrate_image(img): app.function(serialized=True, image=img)(lambda: None) with app.run(client=client): pass - assert len(image._stacked_mounts) == 1 + assert len(image._mount_layers) == 1 hydrate_image(image) - assert len(image._stacked_mounts) == 1 + assert len(image._mount_layers) == 1 image_additional_mount = image.add_local_python_packages("pkg_b") hydrate_image(image_additional_mount) - assert len(image_additional_mount._stacked_mounts) == 2 # another mount added to lazy layer - assert len(image._stacked_mounts) == 1 # original image should not be affected + assert len(image_additional_mount._mount_layers) == 2 # another mount added to lazy layer + assert len(image._mount_layers) == 1 # original image should not be affected image_non_mount = image.run_commands("echo 'hello'") hydrate_image(image_non_mount) - assert len(image_non_mount._stacked_mounts) == 0 + assert len(image_non_mount._mount_layers) == 0 # TODO: assert layers include copy of all mounts + echo layers = get_image_layers(image_non_mount.object_id, servicer) @@ -1136,7 +1136,7 @@ def hydrate_image(img): copy_layer = layers[1] assert copy_layer.dockerfile_commands == ["FROM base", "COPY . /"] - assert copy_layer.context_mount_id == image._stacked_mounts[0].object_id + assert copy_layer.context_mount_id == image._mount_layers[0].object_id copied_files = servicer.mount_contents[copy_layer.context_mount_id].keys() assert len(copied_files) == 8 assert all(fn.startswith("/root/pkg_a/") for fn in copied_files) @@ -1155,7 +1155,7 @@ def test_lazy_mounts_are_attached_to_functions(servicer, client, test_dir, monke fun_def = servicer.app_functions[fun.object_id] added_mounts = set(fun_def.mount_ids) - control_func_mounts assert len(added_mounts) == 1 - assert added_mounts == {img._stacked_mounts[0].object_id} + assert added_mounts == {img._mount_layers[0].object_id} def test_lazy_mounts_are_attached_to_classes(servicer, client, test_dir, monkeypatch): @@ -1175,19 +1175,27 @@ class A: fun_def = servicer.function_by_name("A.*") # class service function added_mounts = set(fun_def.mount_ids) - control_func_mounts assert len(added_mounts) == 1 - assert added_mounts == {img._stacked_mounts[0].object_id} + assert added_mounts == {img._mount_layers[0].object_id} obj = ACls(some_arg="foo") obj.keep_warm(0) # hacky way to force hydration of the *parameter bound* function (instance service function) obj_fun_def = servicer.function_by_name("A.*", ((), {"some_arg": "foo"})) # instance service function added_mounts = set(obj_fun_def.mount_ids) - control_func_mounts assert len(added_mounts) == 1 - assert added_mounts == {img._stacked_mounts[0].object_id} + assert added_mounts == {img._mount_layers[0].object_id} -# def test_lazy_mounts_are_attached_to_sandboxes(servicer, client, test_dir, monkeypatch): -# monkeypatch.syspath_prepend((test_dir / "supports").as_posix()) -# img = Image.debian_slim().add_local_python_packages("pkg_a") +def test_lazy_mounts_are_attached_to_sandboxes(servicer, client, test_dir, monkeypatch): + monkeypatch.syspath_prepend((test_dir / "supports").as_posix()) + deb_slim = Image.debian_slim() + img = deb_slim.add_local_python_packages("pkg_a") + app = App("my-app") + with app.run(client=client): + modal.Sandbox.create(image=img, app=app, client=client) + sandbox_def = servicer.sandbox_defs[0] + + assert sandbox_def.image_id == deb_slim.object_id + assert sandbox_def.mount_ids == [img._mount_layers[0].object_id] # TODO: test build functions w/ lazy mounts From c89b7a47285a3d9f53270833cd676529ec6f6677 Mon Sep 17 00:00:00 2001 From: Elias Freider <elias@modal.com> Date: Wed, 16 Oct 2024 14:22:51 +0200 Subject: [PATCH 08/36] Fix env client issue --- test/image_test.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/image_test.py b/test/image_test.py index 8ee631e12..de7274899 100644 --- a/test/image_test.py +++ b/test/image_test.py @@ -1158,7 +1158,7 @@ def test_lazy_mounts_are_attached_to_functions(servicer, client, test_dir, monke assert added_mounts == {img._mount_layers[0].object_id} -def test_lazy_mounts_are_attached_to_classes(servicer, client, test_dir, monkeypatch): +def test_lazy_mounts_are_attached_to_classes(servicer, client, test_dir, monkeypatch, set_env_client): monkeypatch.syspath_prepend((test_dir / "supports").as_posix()) deb_slim = Image.debian_slim() img = deb_slim.add_local_python_packages("pkg_a") From 244af5751af2f9879f7f0d8f10cde6ec16a00d6a Mon Sep 17 00:00:00 2001 From: Elias Freider <elias@modal.com> Date: Wed, 16 Oct 2024 15:11:17 +0200 Subject: [PATCH 09/36] Disable test on windows --- test/image_test.py | 1 + 1 file changed, 1 insertion(+) diff --git a/test/image_test.py b/test/image_test.py index de7274899..167346d73 100644 --- a/test/image_test.py +++ b/test/image_test.py @@ -1185,6 +1185,7 @@ class A: assert added_mounts == {img._mount_layers[0].object_id} +@skip_windows("servicer sandbox implementation not working on windows") def test_lazy_mounts_are_attached_to_sandboxes(servicer, client, test_dir, monkeypatch): monkeypatch.syspath_prepend((test_dir / "supports").as_posix()) deb_slim = Image.debian_slim() From 813d3f15eb1fdbcd7650fb0a7cbbd5d104e973ef Mon Sep 17 00:00:00 2001 From: Elias Freider <elias@modal.com> Date: Wed, 16 Oct 2024 15:15:47 +0200 Subject: [PATCH 10/36] types --- test/image_test.py | 20 +++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) diff --git a/test/image_test.py b/test/image_test.py index 167346d73..30e0c0758 100644 --- a/test/image_test.py +++ b/test/image_test.py @@ -30,8 +30,8 @@ from .supports.skip import skip_windows -def dummy(): - ... +def dummy() -> None: + return None def test_supported_python_series(): @@ -1147,8 +1147,10 @@ def test_lazy_mounts_are_attached_to_functions(servicer, client, test_dir, monke deb_slim = Image.debian_slim() img = deb_slim.add_local_python_packages("pkg_a") app = App("my-app") - control_fun = app.function(serialized=True, image=deb_slim, name="control")(lambda: None) # no mounts on image - fun = app.function(serialized=True, image=img, name="fun")(lambda: None) # mounts on image + control_fun: modal.Function = app.function(serialized=True, image=deb_slim, name="control")( + dummy + ) # no mounts on image + fun: modal.Function = app.function(serialized=True, image=img, name="fun")(dummy) # mounts on image deploy_app(app, client=client) control_func_mounts = set(servicer.app_functions[control_fun.object_id].mount_ids) @@ -1163,7 +1165,9 @@ def test_lazy_mounts_are_attached_to_classes(servicer, client, test_dir, monkeyp deb_slim = Image.debian_slim() img = deb_slim.add_local_python_packages("pkg_a") app = App("my-app") - control_fun = app.function(serialized=True, image=deb_slim, name="control")(lambda: None) # no mounts on image + control_fun: modal.Function = app.function(serialized=True, image=deb_slim, name="control")( + dummy + ) # no mounts on image class A: some_arg: str = modal.parameter() @@ -1177,8 +1181,10 @@ class A: assert len(added_mounts) == 1 assert added_mounts == {img._mount_layers[0].object_id} - obj = ACls(some_arg="foo") - obj.keep_warm(0) # hacky way to force hydration of the *parameter bound* function (instance service function) + obj = ACls(some_arg="foo") # type: ignore + # hacky way to force hydration of the *parameter bound* function (instance service function): + obj.keep_warm(0) # type: ignore + obj_fun_def = servicer.function_by_name("A.*", ((), {"some_arg": "foo"})) # instance service function added_mounts = set(obj_fun_def.mount_ids) - control_func_mounts assert len(added_mounts) == 1 From 36bc55e4042367de172ff531dfb5a2b37e659e48 Mon Sep 17 00:00:00 2001 From: Elias Freider <elias@modal.com> Date: Mon, 21 Oct 2024 14:34:04 +0200 Subject: [PATCH 11/36] Disable auto mounts in image tests, trims ~30s of test time --- test/image_test.py | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/test/image_test.py b/test/image_test.py index 4e2ad2539..ac6a094d7 100644 --- a/test/image_test.py +++ b/test/image_test.py @@ -30,6 +30,14 @@ from .supports.skip import skip_windows +@pytest.fixture(autouse=True) +def no_automount(monkeypatch): + # no tests in here use automounting, but a lot of them implicitly create + # functions w/ lots of modules is sys.modules which will automount + # which takes a lot of time, so we disable it + monkeypatch.setenv("MODAL_AUTOMOUNT", "0") + + def dummy() -> None: return None From 14907e565b8cae76a02dcc4971011be1917e9c9c Mon Sep 17 00:00:00 2001 From: Elias Freider <elias@modal.com> Date: Tue, 22 Oct 2024 14:27:36 +0200 Subject: [PATCH 12/36] Make materialization an explicit step --- modal/image.py | 42 ++++++++++++++++++++++++++++++++++-------- test/image_test.py | 15 ++++++++++----- 2 files changed, 44 insertions(+), 13 deletions(-) diff --git a/modal/image.py b/modal/image.py index c7d3068bc..3505c7c72 100644 --- a/modal/image.py +++ b/modal/image.py @@ -5,6 +5,7 @@ import re import shlex import sys +import textwrap import typing import warnings from dataclasses import dataclass @@ -305,17 +306,39 @@ def _mount_layers(self) -> typing.Tuple[_Mount]: """ return self._mounts - def _materialize_mounts(self) -> "_Image": - """Takes any stacked mounts on the image and makes them into actual image layers using COPY + def materialize_added_files(self) -> "_Image": + """Builds image layers of all files added through `image.add_*()` methods - This needs to be run before an image with mounts is used as a base image for anything - other than another "mount only" layer, e.g. a docker command. + This is required to run before an non-add_* image operation can be run on the image. + For that reason, it's recommended to always run `iamge.add_*` operations last in the + image build, to avoid having to materialize such layers. """ image = self for mount in self._mount_layers: image = image._materialize_mount(mount) return image + def _assert_materialized(self): + if self._mount_layers: + print("mount layers", self._mount_layers) + raise InvalidError( + textwrap.dedent( + """ + It's recommended to run any `image.add_*` commands for adding local resources last + in your build chain, to prevent having to rebuild images on every local file change. + + If you need local files available in earlier build steps, call image.materialize_added_files() + before adding any non-add image build steps, e.g. + + my_image = ( + Image.debian_slim() + .add_local_python_packages() # an "add" virtual layer + .materialize_added_files() # this copies all virtual layers into the image + .run_commands(...) # this is now ok! + """ + ) + ) + @staticmethod def _from_args( *, @@ -330,12 +353,11 @@ def _from_args( force_build: bool = False, # For internal use only. _namespace: int = api_pb2.DEPLOYMENT_NAMESPACE_WORKSPACE, - _materialize_mounts: bool = True, + _assert_materialized_mounts: bool = True, ): if base_images is None: base_images = {} - else: - base_images = {k: v._materialize_mounts() if _materialize_mounts else v for k, v in base_images.items()} + if secrets is None: secrets = [] if gpu_config is None: @@ -361,6 +383,10 @@ def _deps() -> List[_Object]: return deps async def _load(self: _Image, resolver: Resolver, existing_object_id: Optional[str]): + if _assert_materialized_mounts: + for image in base_images.values(): + image._assert_materialized() + environment = await _get_environment_cached(resolver.environment_name or "", resolver.client) # A bit hacky,but assume that the environment provides a valid builder version image_builder_version = cast(ImageBuilderVersion, environment._settings.image_builder_version) @@ -536,7 +562,7 @@ def build_dockerfile(version: ImageBuilderVersion) -> DockerfileSpec: base_images={"base": self}, dockerfile_function=build_dockerfile, context_mount=mount, - _materialize_mounts=False, # avoid recursion + _assert_materialized_mounts=False, # avoid recursion ) def copy_mount(self, mount: _Mount, remote_path: Union[str, Path] = ".") -> "_Image": diff --git a/test/image_test.py b/test/image_test.py index ac6a094d7..daade7b34 100644 --- a/test/image_test.py +++ b/test/image_test.py @@ -1137,15 +1137,17 @@ def hydrate_image(img): assert len(image_additional_mount._mount_layers) == 2 # another mount added to lazy layer assert len(image._mount_layers) == 1 # original image should not be affected + # running commands image_non_mount = image.run_commands("echo 'hello'") + with pytest.raises(InvalidError, match="image.materialize_added_files"): + hydrate_image(image_non_mount) + + image_non_mount = image.materialize_added_files().run_commands("echo 'hello'") hydrate_image(image_non_mount) + assert len(image_non_mount._mount_layers) == 0 - # TODO: assert layers include copy of all mounts + echo layers = get_image_layers(image_non_mount.object_id, servicer) - for layer in layers: - print("===========LAYER========") - print(layer) echo_layer = layers[0] assert echo_layer.dockerfile_commands == ["FROM base", "RUN echo 'hello'"] @@ -1223,4 +1225,7 @@ def test_lazy_mounts_are_attached_to_sandboxes(servicer, client, test_dir, monke # TODO: test build functions w/ lazy mounts -# TODO: test modal serve w/ lazy mounts +# TODO: test modal serve w/ lazy mounts + materialized lazy mounts (app needs to be rebuilt if mounts +# are changed, even if those mounts are copied into the image) + +# TODO: test modal shell w/ lazy mounts From 19780c7a21cdb724297786c830907e040eccbd79 Mon Sep 17 00:00:00 2001 From: Elias Freider <elias@modal.com> Date: Tue, 22 Oct 2024 14:29:30 +0200 Subject: [PATCH 13/36] comment --- test/image_test.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/test/image_test.py b/test/image_test.py index daade7b34..8c60eb642 100644 --- a/test/image_test.py +++ b/test/image_test.py @@ -1229,3 +1229,5 @@ def test_lazy_mounts_are_attached_to_sandboxes(servicer, client, test_dir, monke # are changed, even if those mounts are copied into the image) # TODO: test modal shell w/ lazy mounts +# this works since the image is passed on as is to a sandbox which will load it and +# transfer any virtual mount layers from the image as mounts to the sandbox From 18d931fccc54b940603ea2a1e32908209c747300 Mon Sep 17 00:00:00 2001 From: Elias Freider <elias@modal.com> Date: Tue, 29 Oct 2024 10:03:35 +0100 Subject: [PATCH 14/36] wip refactor --- modal/image.py | 22 ++++++++-------------- test/image_test.py | 10 +++++----- 2 files changed, 13 insertions(+), 19 deletions(-) diff --git a/modal/image.py b/modal/image.py index 3505c7c72..152ba9ced 100644 --- a/modal/image.py +++ b/modal/image.py @@ -306,18 +306,6 @@ def _mount_layers(self) -> typing.Tuple[_Mount]: """ return self._mounts - def materialize_added_files(self) -> "_Image": - """Builds image layers of all files added through `image.add_*()` methods - - This is required to run before an non-add_* image operation can be run on the image. - For that reason, it's recommended to always run `iamge.add_*` operations last in the - image build, to avoid having to materialize such layers. - """ - image = self - for mount in self._mount_layers: - image = image._materialize_mount(mount) - return image - def _assert_materialized(self): if self._mount_layers: print("mount layers", self._mount_layers) @@ -611,14 +599,20 @@ def build_dockerfile(version: ImageBuilderVersion) -> DockerfileSpec: context_mount=mount, ) - def add_local_python_packages(self, *packages: Union[str, Path]) -> "_Image": + def add_local_python_packages(self, *packages: Union[str, Path], copy: bool = False) -> "_Image": """Adds local Python packages to the image Packages are added to the /root directory which is on the PYTHONPATH of any executed Modal functions. + + Set copy=True to force the package to be added as an image layer rather than + mounted as a light-weight mount (the default). This can be slower since it + requires a rebuild of the image whenever the required package files change + , but it allows you to run additional build steps after this operation. """ mount = _Mount.from_local_python_packages(*packages) - return self._add_mount_layer([mount]) + img = self._add_mount_layer([mount]) + if img._m def copy_local_dir(self, local_path: Union[str, Path], remote_path: Union[str, Path] = ".") -> "_Image": """Copy a directory into the image as a part of building the image. diff --git a/test/image_test.py b/test/image_test.py index 8c60eb642..a17a13a68 100644 --- a/test/image_test.py +++ b/test/image_test.py @@ -1139,15 +1139,15 @@ def hydrate_image(img): # running commands image_non_mount = image.run_commands("echo 'hello'") - with pytest.raises(InvalidError, match="image.materialize_added_files"): + with pytest.raises(InvalidError, match="copy=True"): hydrate_image(image_non_mount) - image_non_mount = image.materialize_added_files().run_commands("echo 'hello'") - hydrate_image(image_non_mount) + image_using_copy = deb.add_local_python_packages("pkg_a", copy=True).run_commands("echo 'hello'") + hydrate_image(image_using_copy) - assert len(image_non_mount._mount_layers) == 0 + assert len(image_using_copy._mount_layers) == 0 - layers = get_image_layers(image_non_mount.object_id, servicer) + layers = get_image_layers(image_using_copy.object_id, servicer) echo_layer = layers[0] assert echo_layer.dockerfile_commands == ["FROM base", "RUN echo 'hello'"] From 01aab3bbb8554d570b81a756640c3cc1b4b956a7 Mon Sep 17 00:00:00 2001 From: Elias Freider <elias@modal.com> Date: Thu, 31 Oct 2024 11:49:13 +0100 Subject: [PATCH 15/36] Fixes --- modal/image.py | 28 +++++++++++++++++----------- 1 file changed, 17 insertions(+), 11 deletions(-) diff --git a/modal/image.py b/modal/image.py index 152ba9ced..7f48edfb2 100644 --- a/modal/image.py +++ b/modal/image.py @@ -284,14 +284,17 @@ def _hydrate_metadata(self, message: Optional[Message]): for exc in self.inside_exceptions: raise exc - def _add_mount_layer(self, mounts: Sequence[_Mount] = ()): + def _add_mount_layer_or_copy(self, mount: _Mount, copy: bool = False): + if copy: + return self.copy_mount(mount, remote_path="/") + base_image = self async def _load(self: _Image, resolver: Resolver, existing_object_id: Optional[str]): self._hydrate_from_other(base_image) # same image id as base image as long as it's lazy - self._mounts = base_image._mounts + tuple(mounts) + self._mounts = base_image._mounts + (mount,) - return _Image._from_loader(_load, "ImageWithMounts()", deps=lambda: [base_image] + list(mounts)) + return _Image._from_loader(_load, "ImageWithMounts()", deps=lambda: [base_image, mount]) @property def _mount_layers(self) -> typing.Tuple[_Mount]: @@ -313,16 +316,20 @@ def _assert_materialized(self): textwrap.dedent( """ It's recommended to run any `image.add_*` commands for adding local resources last - in your build chain, to prevent having to rebuild images on every local file change. + in your build chain to prevent having to rebuild images on every local file change. + Modal then optimizes these files to be added as a thin mount layer when starting your + container, without having the rebuild an image whenever you change the data. + + If you need the files in earlier build steps and are ok with the added build time, + you can explicitly enable that using the `copy=True` option, which copies all + relevant files into the image itself. - If you need local files available in earlier build steps, call image.materialize_added_files() - before adding any non-add image build steps, e.g. + E.g: my_image = ( Image.debian_slim() - .add_local_python_packages() # an "add" virtual layer - .materialize_added_files() # this copies all virtual layers into the image - .run_commands(...) # this is now ok! + .add_local_python_packages("mypak", copy=True) + .run_commands("python -m mypak") # this is now ok! """ ) ) @@ -611,8 +618,7 @@ def add_local_python_packages(self, *packages: Union[str, Path], copy: bool = Fa , but it allows you to run additional build steps after this operation. """ mount = _Mount.from_local_python_packages(*packages) - img = self._add_mount_layer([mount]) - if img._m + return self._add_mount_layer_or_copy(mount, copy=copy) def copy_local_dir(self, local_path: Union[str, Path], remote_path: Union[str, Path] = ".") -> "_Image": """Copy a directory into the image as a part of building the image. From 0abd75b9ceb6bdce4a6c410c0bc3538713d7c097 Mon Sep 17 00:00:00 2001 From: Elias Freider <elias@modal.com> Date: Thu, 31 Oct 2024 13:24:54 +0100 Subject: [PATCH 16/36] Fixing tests --- modal/image.py | 1 - test/image_test.py | 100 ++++++++++++++++++++++++++++++--------------- 2 files changed, 66 insertions(+), 35 deletions(-) diff --git a/modal/image.py b/modal/image.py index d768f48d5..cfe2e9ff4 100644 --- a/modal/image.py +++ b/modal/image.py @@ -313,7 +313,6 @@ def _mount_layers(self) -> typing.Tuple[_Mount]: def _assert_materialized(self): if self._mount_layers: - print("mount layers", self._mount_layers) raise InvalidError( textwrap.dedent( """ diff --git a/test/image_test.py b/test/image_test.py index 59b467f47..d8084d23f 100644 --- a/test/image_test.py +++ b/test/image_test.py @@ -286,8 +286,6 @@ def test_image_pip_install_pyproject(builder_version, servicer, client): app.function(image=image)(dummy) with app.run(client=client): layers = get_image_layers(image.object_id, servicer) - - print(layers[0].dockerfile_commands) assert any("pip install 'banana >=1.2.0' 'potato >=0.1.0'" in cmd for cmd in layers[0].dockerfile_commands) @@ -300,7 +298,6 @@ def test_image_pip_install_pyproject_with_optionals(builder_version, servicer, c with app.run(client=client): layers = get_image_layers(image.object_id, servicer) - print(layers[0].dockerfile_commands) assert any( "pip install 'banana >=1.2.0' 'linting-tool >=0.0.0' 'potato >=0.1.0' 'pytest >=1.2.0'" in cmd for cmd in layers[0].dockerfile_commands @@ -1121,52 +1118,60 @@ async def test_logs(servicer, client): assert logs == ["build starting\n", "build finished\n"] -def test_add_local_python_packages(client, servicer, set_env_client, test_dir, monkeypatch): +@pytest.fixture() +def supports_on_path(test_dir, monkeypatch): monkeypatch.syspath_prepend((test_dir / "supports").as_posix()) - deb = Image.debian_slim() - image = deb.add_local_python_packages("pkg_a") - def hydrate_image(img): - # there should be a more straight forward way to do this? - app = App() - app.function(serialized=True, image=img)(lambda: None) - with app.run(client=client): - pass - assert len(image._mount_layers) == 1 - hydrate_image(image) - assert len(image._mount_layers) == 1 +def hydrate_image(img, client): + # there should be a more straight forward way to do this? + app = App() + app.function(serialized=True, image=img)(lambda: None) + with app.run(client=client): + pass + - image_additional_mount = image.add_local_python_packages("pkg_b") - hydrate_image(image_additional_mount) +def test_add_local_lazy_vs_not(client, servicer, set_env_client, supports_on_path): + deb = Image.debian_slim() + image_with_mount = deb.add_local_python_packages("pkg_a") + + hydrate_image(image_with_mount, client) + assert image_with_mount.object_id == deb.object_id + assert len(image_with_mount._mount_layers) == 1 + + image_additional_mount = image_with_mount.add_local_python_packages("pkg_b") + hydrate_image(image_additional_mount, client) assert len(image_additional_mount._mount_layers) == 2 # another mount added to lazy layer - assert len(image._mount_layers) == 1 # original image should not be affected + assert len(image_with_mount._mount_layers) == 1 # original image should not be affected # running commands - image_non_mount = image.run_commands("echo 'hello'") + image_non_mount = image_with_mount.run_commands("echo 'hello'") with pytest.raises(InvalidError, match="copy=True"): - hydrate_image(image_non_mount) + # error about using non-copy add commands before other build steps + hydrate_image(image_non_mount, client) - image_using_copy = deb.add_local_python_packages("pkg_a", copy=True).run_commands("echo 'hello'") - hydrate_image(image_using_copy) + image_with_copy = deb.add_local_python_packages("pkg_a", copy=True) + hydrate_image(image_with_copy, client) + assert len(image_with_copy._mount_layers) == 0 - assert len(image_using_copy._mount_layers) == 0 + # do the same exact image using copy=True + image_with_copy_and_commands = deb.add_local_python_packages("pkg_a", copy=True).run_commands("echo 'hello'") + hydrate_image(image_with_copy_and_commands, client) + assert len(image_with_copy_and_commands._mount_layers) == 0 - layers = get_image_layers(image_using_copy.object_id, servicer) + layers = get_image_layers(image_with_copy_and_commands.object_id, servicer) echo_layer = layers[0] assert echo_layer.dockerfile_commands == ["FROM base", "RUN echo 'hello'"] copy_layer = layers[1] assert copy_layer.dockerfile_commands == ["FROM base", "COPY . /"] - assert copy_layer.context_mount_id == image._mount_layers[0].object_id copied_files = servicer.mount_contents[copy_layer.context_mount_id].keys() assert len(copied_files) == 8 assert all(fn.startswith("/root/pkg_a/") for fn in copied_files) -def test_lazy_mounts_are_attached_to_functions(servicer, client, test_dir, monkeypatch): - monkeypatch.syspath_prepend((test_dir / "supports").as_posix()) +def test_add_local_mount_are_attached_to_functions(servicer, client, supports_on_path): deb_slim = Image.debian_slim() img = deb_slim.add_local_python_packages("pkg_a") app = App("my-app") @@ -1183,8 +1188,7 @@ def test_lazy_mounts_are_attached_to_functions(servicer, client, test_dir, monke assert added_mounts == {img._mount_layers[0].object_id} -def test_lazy_mounts_are_attached_to_classes(servicer, client, test_dir, monkeypatch, set_env_client): - monkeypatch.syspath_prepend((test_dir / "supports").as_posix()) +def test_add_local_mount_are_attached_to_classes(servicer, client, supports_on_path, set_env_client): deb_slim = Image.debian_slim() img = deb_slim.add_local_python_packages("pkg_a") app = App("my-app") @@ -1215,8 +1219,7 @@ class A: @skip_windows("servicer sandbox implementation not working on windows") -def test_lazy_mounts_are_attached_to_sandboxes(servicer, client, test_dir, monkeypatch): - monkeypatch.syspath_prepend((test_dir / "supports").as_posix()) +def test_add_local_mount_are_attached_to_sandboxes(servicer, client, supports_on_path): deb_slim = Image.debian_slim() img = deb_slim.add_local_python_packages("pkg_a") app = App("my-app") @@ -1226,12 +1229,41 @@ def test_lazy_mounts_are_attached_to_sandboxes(servicer, client, test_dir, monke assert sandbox_def.image_id == deb_slim.object_id assert sandbox_def.mount_ids == [img._mount_layers[0].object_id] + copied_files = servicer.mount_contents[sandbox_def.mount_ids[0]] + assert len(copied_files) == 8 + assert all(fn.startswith("/root/pkg_a/") for fn in copied_files) -# TODO: test build functions w/ lazy mounts +def empty_fun(): + pass + + +def test_add_local_mount_build_function(servicer, client, supports_on_path): + deb_slim = Image.debian_slim() + img = deb_slim.add_local_python_packages("pkg_a") + img_with_build_function = img.run_function(empty_fun) + with pytest.raises(InvalidError): + # build functions could still potentially rewrite mount contents, + # so we still require them to use copy=True + # TODO(elias): what if someone wants do use an equivalent of `run_function(..., mounts=[...]) ? + hydrate_image(img_with_build_function, client) + + img_with_copy = deb_slim.add_local_python_packages("pkg_a", copy=True) + hydrate_image(img_with_copy, client) # this is fine + + +def test_add_local_mount_included_in_serve_watchers(servicer, client, supports_on_path): + deb_slim = Image.debian_slim() + img = deb_slim.add_local_python_packages("pkg_a") + app = App() + + @app.function(serialized=True, image=img) + def f(): + pass + + watch_mounts = app._get_watch_mounts() + assert watch_mounts -# TODO: test modal serve w/ lazy mounts + materialized lazy mounts (app needs to be rebuilt if mounts -# are changed, even if those mounts are copied into the image) # TODO: test modal shell w/ lazy mounts # this works since the image is passed on as is to a sandbox which will load it and From 6e2db2448d5c90c3b9e9d864c55933776ab750c3 Mon Sep 17 00:00:00 2001 From: Elias Freider <elias@modal.com> Date: Thu, 31 Oct 2024 14:19:16 +0100 Subject: [PATCH 17/36] Adds more watcher tests, including failing one --- modal/app.py | 3 +++ test/conftest.py | 6 +++++ test/watcher_test.py | 62 ++++++++++++++++++++++++++++++++------------ 3 files changed, 55 insertions(+), 16 deletions(-) diff --git a/modal/app.py b/modal/app.py index e44f8f3f1..98c455771 100644 --- a/modal/app.py +++ b/modal/app.py @@ -478,6 +478,9 @@ def _get_default_image(self): return _default_image def _get_watch_mounts(self): + if not self._running_app: + raise InvalidError("`_get_watch_mounts` requires a running app.") + all_mounts = [ *self._mounts, ] diff --git a/test/conftest.py b/test/conftest.py index a60f88b70..dab63efca 100644 --- a/test/conftest.py +++ b/test/conftest.py @@ -1960,3 +1960,9 @@ async def set_env_client(client): yield finally: Client.set_env_client(None) + + +@pytest.fixture +def disable_auto_mount(monkeypatch): + monkeypatch.setenv("MODAL_AUTOMOUNT", "0") + yield diff --git a/test/watcher_test.py b/test/watcher_test.py index dec89c4c8..cf736f222 100644 --- a/test/watcher_test.py +++ b/test/watcher_test.py @@ -2,14 +2,14 @@ import pytest import random import string -import sys from pathlib import Path from watchfiles import Change import modal from modal._watcher import _watch_args_from_mounts -from modal.mount import Mount, _Mount +from modal.exception import InvalidError +from modal.mount import _get_client_mount, _Mount @pytest.mark.asyncio @@ -36,23 +36,53 @@ def dummy(): pass -@pytest.fixture() -def clean_sys_modules(monkeypatch): - # run test assuming no user-defined modules have been loaded - module_names = set() - for name, mod in sys.modules.items(): - if getattr(mod, "__file__", None) and not ("/lib/" in mod.__file__ or "/site-packages/" in mod.__file__): - module_names.add(name) +def test_watch_mounts_requires_running_app(): + # requires running app to make sure the mounts have been loaded + app = modal.App() + with pytest.raises(InvalidError): + # _get_watch_mounts needs to be called on a hydrated app + app._get_watch_mounts() + + +def test_watch_mounts_includes_function_mounts(client, supports_dir, monkeypatch, disable_auto_mount): + monkeypatch.syspath_prepend(supports_dir) + app = modal.App() + pkg_a_mount = modal.Mount.from_local_python_packages("pkg_a") + + @app.function(mounts=[pkg_a_mount], serialized=True) + def f(): + pass + + with app.run(client=client): + watch_mounts = app._get_watch_mounts() + assert watch_mounts == [pkg_a_mount] - for m in module_names: - monkeypatch.delitem(sys.modules, m) +def test_watch_mounts_includes_image_mounts(client, supports_dir, monkeypatch, disable_auto_mount): + monkeypatch.syspath_prepend(supports_dir) + app = modal.App() + pkg_a_mount = modal.Mount.from_local_python_packages("pkg_a") + image = modal.Image.debian_slim().copy_mount(pkg_a_mount) + + @app.function(image=image, serialized=True) + def f(): + pass + + with app.run(client=client): + watch_mounts = app._get_watch_mounts() + assert watch_mounts == [pkg_a_mount] -@pytest.mark.usefixtures("clean_sys_modules") -@pytest.mark.skip("not working in ci for some reason. deactivating for now") # TODO(elias) fix -def test_watch_mounts_ignore_local(): + +def test_watch_mounts_ignore_non_local(disable_auto_mount, client, servicer): app = modal.App() - app.function(mounts=[Mount.from_name("some-published-mount")])(dummy) - mounts = app._get_watch_mounts() + # uses the published client mount - should not be included in watch items + # serialized=True avoids auto-mounting the entrypoint + @app.function(mounts=[_get_client_mount()], serialized=True) + def dummy(): + pass + + with app.run(client=client): + mounts = app._get_watch_mounts() + assert len(mounts) == 0 From 2ffac62d92688907bb31eff1f6ebb847ea907cda Mon Sep 17 00:00:00 2001 From: Elias Freider <elias@modal.com> Date: Thu, 31 Oct 2024 19:25:20 +0100 Subject: [PATCH 18/36] wip --- modal/image.py | 1 + 1 file changed, 1 insertion(+) diff --git a/modal/image.py b/modal/image.py index f03c33874..b40e45454 100644 --- a/modal/image.py +++ b/modal/image.py @@ -266,6 +266,7 @@ class _Image(_Object, type_prefix="im"): force_build: bool inside_exceptions: List[Exception] + _used_local_mounts: Optional[Sequence[_Mount]] = () # used for mounts watching def _initialize_from_empty(self): self.inside_exceptions = [] From 6b107a102b86af3c844347702b177b1b1e25b51c Mon Sep 17 00:00:00 2001 From: Elias Freider <elias@modal.com> Date: Wed, 6 Nov 2024 14:30:35 +0100 Subject: [PATCH 19/36] Move mount extraction logic post-app-load, include image mounts --- modal/app.py | 4 ++-- modal/functions.py | 9 +++------ modal/image.py | 8 +++++++- modal/serving.py | 11 +++++------ 4 files changed, 17 insertions(+), 15 deletions(-) diff --git a/modal/app.py b/modal/app.py index 98c455771..042f572e7 100644 --- a/modal/app.py +++ b/modal/app.py @@ -165,7 +165,7 @@ def foo(): _name: Optional[str] _description: Optional[str] _indexed_objects: Dict[str, _Object] - _function_mounts: Dict[str, _Mount] + _image: Optional[_Image] _mounts: Sequence[_Mount] _secrets: Sequence[_Secret] @@ -485,7 +485,7 @@ def _get_watch_mounts(self): *self._mounts, ] for function in self.registered_functions.values(): - all_mounts.extend(function._all_mounts) + all_mounts.extend(function._used_local_mounts) return [m for m in all_mounts if m.is_local()] diff --git a/modal/functions.py b/modal/functions.py index 6d614901c..a402b448f 100644 --- a/modal/functions.py +++ b/modal/functions.py @@ -305,7 +305,7 @@ class _Function(typing.Generic[P, ReturnType, OriginalReturnType], _Object, type # TODO: more type annotations _info: Optional[FunctionInfo] - _all_mounts: Collection[_Mount] + _used_local_mounts: typing.Set[_Mount] _app: Optional["modal.app._App"] = None _obj: Optional["modal.cls._Obj"] = None # only set for InstanceServiceFunctions and bound instance methods _web_url: Optional[str] @@ -418,7 +418,6 @@ def _deps(): fun._use_method_name = method_name fun._app = class_service_function._app fun._is_generator = partial_function.is_generator - fun._all_mounts = class_service_function._all_mounts fun._spec = class_service_function._spec fun._is_method = True return fun @@ -483,7 +482,6 @@ def _deps(): fun._is_method = True fun._parent = instance_service_function._parent fun._app = class_bound_method._app - fun._all_mounts = class_bound_method._all_mounts # TODO: only used for mount-watching/modal serve fun._spec = class_bound_method._spec return fun @@ -927,7 +925,8 @@ async def _load(self: _Function, resolver: Resolver, existing_object_id: Optiona raise InvalidError(f"Function {info.function_name} is too large to deploy.") raise function_creation_status.set_response(response) - + obj._used_local_mounts = set(m for m in all_mounts if m.is_local()) # needed for modal.serve file watching + obj._used_local_mounts |= image._used_local_mounts self._hydrate(response.function_id, resolver.client, response.handle_metadata) rep = f"Function({tag})" @@ -936,7 +935,6 @@ async def _load(self: _Function, resolver: Resolver, existing_object_id: Optiona obj._raw_f = info.raw_f obj._info = info obj._tag = tag - obj._all_mounts = all_mounts # needed for modal.serve file watching obj._app = app # needed for CLI right now obj._obj = None obj._is_generator = is_generator @@ -1169,7 +1167,6 @@ def _initialize_from_empty(self): self._web_url = None self._function_name = None self._info = None - self._all_mounts = [] # used for file watching self._use_function_id = "" def _hydrate_metadata(self, metadata: Optional[Message]): diff --git a/modal/image.py b/modal/image.py index b40e45454..e6a25a22d 100644 --- a/modal/image.py +++ b/modal/image.py @@ -266,10 +266,11 @@ class _Image(_Object, type_prefix="im"): force_build: bool inside_exceptions: List[Exception] - _used_local_mounts: Optional[Sequence[_Mount]] = () # used for mounts watching + _used_local_mounts: Set[_Mount] # used for mounts watching def _initialize_from_empty(self): self.inside_exceptions = [] + self._used_local_mounts = set() def _hydrate_metadata(self, message: Optional[Message]): env_image_id = config.get("image_id") @@ -462,6 +463,11 @@ async def join(): raise RemoteError("Unknown status %s!" % result.status) self._hydrate(image_id, resolver.client, None) + self._used_local_mounts = set() + for base in base_images.values(): + self._used_local_mounts |= base._used_local_mounts + if context_mount: + self._used_local_mounts.add(context_mount) rep = "Image()" obj = _Image._from_loader(_load, rep, deps=_deps) diff --git a/modal/serving.py b/modal/serving.py index e9ef60692..da2e4ac14 100644 --- a/modal/serving.py +++ b/modal/serving.py @@ -104,13 +104,12 @@ async def _serve_app( client = await _Client.from_env() - if _watcher is not None: - watcher = _watcher # Only used by tests - else: - mounts_to_watch = app._get_watch_mounts() - watcher = watch(mounts_to_watch) - async with _run_app(app, client=client, environment_name=environment_name): + if _watcher is not None: + watcher = _watcher # Only used by tests + else: + mounts_to_watch = app._get_watch_mounts() + watcher = watch(mounts_to_watch) async with TaskContext(grace=0.1) as tc: tc.create_task(_run_watch_loop(app_ref, app.app_id, watcher, environment_name)) yield app From dfc627b09ed1f0969afe37bb43d0d26fa4f3ca10 Mon Sep 17 00:00:00 2001 From: Elias Freider <elias@modal.com> Date: Wed, 6 Nov 2024 14:33:44 +0100 Subject: [PATCH 20/36] if --- modal/image.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modal/image.py b/modal/image.py index e6a25a22d..4f4ce1027 100644 --- a/modal/image.py +++ b/modal/image.py @@ -466,7 +466,7 @@ async def join(): self._used_local_mounts = set() for base in base_images.values(): self._used_local_mounts |= base._used_local_mounts - if context_mount: + if context_mount and context_mount.is_local(): self._used_local_mounts.add(context_mount) rep = "Image()" From 908b329d48e68fe04b69fcfa41d8962803a79a90 Mon Sep 17 00:00:00 2001 From: Elias Freider <elias@modal.com> Date: Wed, 6 Nov 2024 15:37:48 +0100 Subject: [PATCH 21/36] Add test + fix issue with classes --- modal/functions.py | 8 +++++--- modal/image.py | 11 ++++++----- test/watcher_test.py | 17 +++++++++++++++++ 3 files changed, 28 insertions(+), 8 deletions(-) diff --git a/modal/functions.py b/modal/functions.py index a402b448f..b70a0c6a3 100644 --- a/modal/functions.py +++ b/modal/functions.py @@ -305,7 +305,7 @@ class _Function(typing.Generic[P, ReturnType, OriginalReturnType], _Object, type # TODO: more type annotations _info: Optional[FunctionInfo] - _used_local_mounts: typing.Set[_Mount] + _used_local_mounts: typing.FrozenSet[_Mount] # set at load time, only by loader _app: Optional["modal.app._App"] = None _obj: Optional["modal.cls._Obj"] = None # only set for InstanceServiceFunctions and bound instance methods _web_url: Optional[str] @@ -925,8 +925,9 @@ async def _load(self: _Function, resolver: Resolver, existing_object_id: Optiona raise InvalidError(f"Function {info.function_name} is too large to deploy.") raise function_creation_status.set_response(response) - obj._used_local_mounts = set(m for m in all_mounts if m.is_local()) # needed for modal.serve file watching - obj._used_local_mounts |= image._used_local_mounts + local_mounts = set(m for m in all_mounts if m.is_local()) # needed for modal.serve file watching + local_mounts |= image._used_local_mounts + obj._used_local_mounts = frozenset(local_mounts) self._hydrate(response.function_id, resolver.client, response.handle_metadata) rep = f"Function({tag})" @@ -1168,6 +1169,7 @@ def _initialize_from_empty(self): self._function_name = None self._info = None self._use_function_id = "" + self._used_local_mounts = frozenset() def _hydrate_metadata(self, metadata: Optional[Message]): # Overridden concrete implementation of base class method diff --git a/modal/image.py b/modal/image.py index 4f4ce1027..22dccfa3d 100644 --- a/modal/image.py +++ b/modal/image.py @@ -266,11 +266,11 @@ class _Image(_Object, type_prefix="im"): force_build: bool inside_exceptions: List[Exception] - _used_local_mounts: Set[_Mount] # used for mounts watching + _used_local_mounts: typing.FrozenSet[_Mount] # used for mounts watching def _initialize_from_empty(self): self.inside_exceptions = [] - self._used_local_mounts = set() + self._used_local_mounts = frozenset() def _hydrate_metadata(self, message: Optional[Message]): env_image_id = config.get("image_id") @@ -463,11 +463,12 @@ async def join(): raise RemoteError("Unknown status %s!" % result.status) self._hydrate(image_id, resolver.client, None) - self._used_local_mounts = set() + local_mounts = set() for base in base_images.values(): - self._used_local_mounts |= base._used_local_mounts + local_mounts |= base._used_local_mounts if context_mount and context_mount.is_local(): - self._used_local_mounts.add(context_mount) + local_mounts.add(context_mount) + self._used_local_mounts = frozenset(local_mounts) rep = "Image()" obj = _Image._from_loader(_load, rep, deps=_deps) diff --git a/test/watcher_test.py b/test/watcher_test.py index cf736f222..3aacc2b5a 100644 --- a/test/watcher_test.py +++ b/test/watcher_test.py @@ -7,6 +7,7 @@ from watchfiles import Change import modal +from modal import method from modal._watcher import _watch_args_from_mounts from modal.exception import InvalidError from modal.mount import _get_client_mount, _Mount @@ -58,6 +59,22 @@ def f(): assert watch_mounts == [pkg_a_mount] +def test_watch_mounts_includes_cls_mounts(client, supports_dir, monkeypatch, disable_auto_mount): + monkeypatch.syspath_prepend(supports_dir) + app = modal.App() + pkg_a_mount = modal.Mount.from_local_python_packages("pkg_a") + + @app.cls(mounts=[pkg_a_mount], serialized=True) + class A: + @method() + def foo(self): + pass + + with app.run(client=client): + watch_mounts = app._get_watch_mounts() + assert watch_mounts == [pkg_a_mount] + + def test_watch_mounts_includes_image_mounts(client, supports_dir, monkeypatch, disable_auto_mount): monkeypatch.syspath_prepend(supports_dir) app = modal.App() From 6b954c9c15e26b2677e5d7968391aeb7b2feac93 Mon Sep 17 00:00:00 2001 From: Elias Freider <elias@modal.com> Date: Wed, 6 Nov 2024 16:15:48 +0100 Subject: [PATCH 22/36] Fix tests --- modal/image.py | 31 ++++++++++--------------------- test/conftest.py | 5 +++++ test/image_test.py | 18 ------------------ test/watcher_test.py | 14 ++++++++++++++ 4 files changed, 29 insertions(+), 39 deletions(-) diff --git a/modal/image.py b/modal/image.py index 832c5b9fe..28a94909a 100644 --- a/modal/image.py +++ b/modal/image.py @@ -276,8 +276,7 @@ class _Image(_Object, type_prefix="im"): force_build: bool inside_exceptions: List[Exception] _used_local_mounts: typing.FrozenSet[_Mount] # used for mounts watching - _used_local_mounts: Set[_Mount] # used for mounts watching - _mounts: Sequence[_Mount] + _mounts: Sequence[_Mount] # added as mounts on any container referencing the Image, see `def _mount_layers` def _initialize_from_empty(self): self.inside_exceptions = [] @@ -304,9 +303,10 @@ def _add_mount_layer_or_copy(self, mount: _Mount, copy: bool = False): base_image = self - async def _load(self: _Image, resolver: Resolver, existing_object_id: Optional[str]): - self._hydrate_from_other(base_image) # same image id as base image as long as it's lazy - self._mounts = base_image._mounts + (mount,) + async def _load(self2: "_Image", resolver: Resolver, existing_object_id: Optional[str]): + self2._hydrate_from_other(base_image) # same image id as base image as long as it's lazy + self2._mounts = tuple(base_image._mounts) + (mount,) + self2._used_local_mounts = base_image._used_local_mounts | ({mount} if mount.is_local() else set()) return _Image._from_loader(_load, "ImageWithMounts()", deps=lambda: [base_image, mount]) @@ -323,7 +323,7 @@ def _mount_layers(self) -> typing.Tuple[_Mount]: """ return self._mounts - def _assert_materialized(self): + def _assert_no_mount_layers(self): if self._mount_layers: raise InvalidError( textwrap.dedent( @@ -361,7 +361,7 @@ def _from_args( force_build: bool = False, # For internal use only. _namespace: int = api_pb2.DEPLOYMENT_NAMESPACE_WORKSPACE, - _assert_materialized_mounts: bool = True, + _do_assert_no_mount_layers: bool = True, ): if base_images is None: base_images = {} @@ -391,9 +391,10 @@ def _deps() -> List[_Object]: return deps async def _load(self: _Image, resolver: Resolver, existing_object_id: Optional[str]): - if _assert_materialized_mounts: + if _do_assert_no_mount_layers: for image in base_images.values(): - image._assert_materialized() + # base images can't have + image._assert_no_mount_layers() environment = await _get_environment_cached(resolver.environment_name or "", resolver.client) # A bit hacky,but assume that the environment provides a valid builder version @@ -567,18 +568,6 @@ def build_dockerfile(version: ImageBuilderVersion) -> DockerfileSpec: return _Image._from_args(base_images={"base": self}, dockerfile_function=build_dockerfile, **kwargs) - def _materialize_mount(self, mount: _Mount) -> "_Image": - def build_dockerfile(version: ImageBuilderVersion) -> DockerfileSpec: - commands = ["FROM base", "COPY . /"] # copy everything from the supplied mount into the root - return DockerfileSpec(commands=commands, context_files={}) - - return _Image._from_args( - base_images={"base": self}, - dockerfile_function=build_dockerfile, - context_mount=mount, - _assert_materialized_mounts=False, # avoid recursion - ) - def copy_mount(self, mount: _Mount, remote_path: Union[str, Path] = ".") -> "_Image": """Copy the entire contents of a `modal.Mount` into an image. Useful when files only available locally are required during the image diff --git a/test/conftest.py b/test/conftest.py index 74c219b23..caa0d35d2 100644 --- a/test/conftest.py +++ b/test/conftest.py @@ -1970,3 +1970,8 @@ async def set_env_client(client): def disable_auto_mount(monkeypatch): monkeypatch.setenv("MODAL_AUTOMOUNT", "0") yield + + +@pytest.fixture() +def supports_on_path(supports_dir, monkeypatch): + monkeypatch.syspath_prepend(str(supports_dir)) diff --git a/test/image_test.py b/test/image_test.py index 58ff2e53e..32fb8ad96 100644 --- a/test/image_test.py +++ b/test/image_test.py @@ -1193,11 +1193,6 @@ async def test_logs(servicer, client): assert logs == ["build starting\n", "build finished\n"] -@pytest.fixture() -def supports_on_path(test_dir, monkeypatch): - monkeypatch.syspath_prepend((test_dir / "supports").as_posix()) - - def hydrate_image(img, client): # there should be a more straight forward way to do this? app = App() @@ -1327,19 +1322,6 @@ def test_add_local_mount_build_function(servicer, client, supports_on_path): hydrate_image(img_with_copy, client) # this is fine -def test_add_local_mount_included_in_serve_watchers(servicer, client, supports_on_path): - deb_slim = Image.debian_slim() - img = deb_slim.add_local_python_packages("pkg_a") - app = App() - - @app.function(serialized=True, image=img) - def f(): - pass - - watch_mounts = app._get_watch_mounts() - assert watch_mounts - - # TODO: test modal shell w/ lazy mounts # this works since the image is passed on as is to a sandbox which will load it and # transfer any virtual mount layers from the image as mounts to the sandbox diff --git a/test/watcher_test.py b/test/watcher_test.py index 3aacc2b5a..5df830a34 100644 --- a/test/watcher_test.py +++ b/test/watcher_test.py @@ -103,3 +103,17 @@ def dummy(): mounts = app._get_watch_mounts() assert len(mounts) == 0 + + +def test_add_local_mount_included_in_serve_watchers(servicer, client, supports_on_path, disable_auto_mount): + deb_slim = modal.Image.debian_slim() + img = deb_slim.add_local_python_packages("pkg_a") + app = modal.App() + + @app.function(serialized=True, image=img) + def f(): + pass + + with app.run(client=client): + watch_mounts = app._get_watch_mounts() + assert len(watch_mounts) == 1 From 8f81737bfc16b72b988df7541b6c3040db5b6245 Mon Sep 17 00:00:00 2001 From: Elias Freider <elias@modal.com> Date: Wed, 6 Nov 2024 16:35:12 +0100 Subject: [PATCH 23/36] Cleanup --- modal/functions.py | 2 +- modal/image.py | 3 --- 2 files changed, 1 insertion(+), 4 deletions(-) diff --git a/modal/functions.py b/modal/functions.py index 157e68e85..d0aa9cf13 100644 --- a/modal/functions.py +++ b/modal/functions.py @@ -575,7 +575,7 @@ def from_args( if proxy: # HACK: remove this once we stop using ssh tunnels for this. if image: - # TODO(elias): this breaks lazy mounts by materializing them, which isn't great + # TODO(elias): this will cause an error if users use prior `.add_*` commands without copy=True image = image.apt_install("autossh") function_spec = _FunctionSpec( diff --git a/modal/image.py b/modal/image.py index 28a94909a..6ad073e35 100644 --- a/modal/image.py +++ b/modal/image.py @@ -1681,8 +1681,5 @@ async def _logs(self) -> AsyncGenerator[str, None]: if task_log.data: yield task_log.data - def _layers(self) -> typing.Tuple["_Image"]: - pass - Image = synchronize_api(_Image) From 7ccb07ab11846c58728a72c272b2245655566f7b Mon Sep 17 00:00:00 2001 From: Elias Freider <elias@modal.com> Date: Wed, 6 Nov 2024 16:48:15 +0100 Subject: [PATCH 24/36] Copy --- modal/image.py | 32 ++++++++++++++++---------------- 1 file changed, 16 insertions(+), 16 deletions(-) diff --git a/modal/image.py b/modal/image.py index 6ad073e35..c579d3c89 100644 --- a/modal/image.py +++ b/modal/image.py @@ -328,22 +328,22 @@ def _assert_no_mount_layers(self): raise InvalidError( textwrap.dedent( """ - It's recommended to run any `image.add_*` commands for adding local resources last - in your build chain to prevent having to rebuild images on every local file change. - Modal then optimizes these files to be added as a thin mount layer when starting your - container, without having the rebuild an image whenever you change the data. - - If you need the files in earlier build steps and are ok with the added build time, - you can explicitly enable that using the `copy=True` option, which copies all - relevant files into the image itself. - - E.g: - - my_image = ( - Image.debian_slim() - .add_local_python_packages("mypak", copy=True) - .run_commands("python -m mypak") # this is now ok! - """ + An image tried to run a build step after using `image.add_*` to include local files. + + Run `image.add_*` commands last in your image build to avoid rebuilding images with every local filechange. + Modal will then mount these files as a thin layer when starting your container, saving build time. + + If you need these files earlier in the build, set `copy=True` to copy the files directly into the image, + though this will increase build time. + + Example: + + my_image = ( + Image.debian_slim() + .add_local_python_packages("mypak", copy=True) + .run_commands("python -m mypak") # this now works! + ) + """ ) ) From fd39606b1c0d9b66a1a80a7a881c2338cfd32a63 Mon Sep 17 00:00:00 2001 From: Elias Freider <elias@modal.com> Date: Fri, 15 Nov 2024 10:38:34 +0100 Subject: [PATCH 25/36] Rename to attach --- modal/image.py | 17 ++++++++++------- test/watcher_test.py | 2 +- 2 files changed, 11 insertions(+), 8 deletions(-) diff --git a/modal/image.py b/modal/image.py index 3b8bdb88f..543e7d51c 100644 --- a/modal/image.py +++ b/modal/image.py @@ -340,7 +340,7 @@ def _assert_no_mount_layers(self): my_image = ( Image.debian_slim() - .add_local_python_packages("mypak", copy=True) + .attach_local_python_packages("mypak", copy=True) .run_commands("python -m mypak") # this now works! ) """ @@ -614,16 +614,19 @@ def build_dockerfile(version: ImageBuilderVersion) -> DockerfileSpec: context_mount=mount, ) - def add_local_python_packages(self, *packages: Union[str, Path], copy: bool = False) -> "_Image": - """Adds local Python packages to the image + def attach_local_python_packages(self, *packages: Union[str, Path], copy: bool = False) -> "_Image": + """Attaches local Python packages to the container running the image Packages are added to the /root directory which is on the PYTHONPATH of any executed Modal functions. - Set copy=True to force the package to be added as an image layer rather than - mounted as a light-weight mount (the default). This can be slower since it - requires a rebuild of the image whenever the required package files change - , but it allows you to run additional build steps after this operation. + By default (copy=False) the packages are layered on top of the image when + the container starts up and not built as an image layer. + + Set copy=True to force the packages to be added as an image layer instead. + This can be slower since it requires a rebuild of the image whenever the + required package files change, but it allows you to run additional build + steps after this operation. """ mount = _Mount.from_local_python_packages(*packages) return self._add_mount_layer_or_copy(mount, copy=copy) diff --git a/test/watcher_test.py b/test/watcher_test.py index f4ea014d8..045d8f62f 100644 --- a/test/watcher_test.py +++ b/test/watcher_test.py @@ -112,7 +112,7 @@ def dummy(): def test_add_local_mount_included_in_serve_watchers(servicer, client, supports_on_path, disable_auto_mount): deb_slim = modal.Image.debian_slim() - img = deb_slim.add_local_python_packages("pkg_a") + img = deb_slim.attach_local_python_packages("pkg_a") app = modal.App() @app.function(serialized=True, image=img) From d3a4dec2848eff461491f12a20e45428497b9754 Mon Sep 17 00:00:00 2001 From: Elias Freider <elias@modal.com> Date: Fri, 15 Nov 2024 10:43:39 +0100 Subject: [PATCH 26/36] Copy --- modal/functions.py | 2 +- modal/image.py | 7 ++++--- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/modal/functions.py b/modal/functions.py index e1ea8d1ce..699094080 100644 --- a/modal/functions.py +++ b/modal/functions.py @@ -578,7 +578,7 @@ def from_args( if proxy: # HACK: remove this once we stop using ssh tunnels for this. if image: - # TODO(elias): this will cause an error if users use prior `.add_*` commands without copy=True + # TODO(elias): this will cause an error if users use prior `.attach_local_*` commands without copy=True image = image.apt_install("autossh") function_spec = _FunctionSpec( diff --git a/modal/image.py b/modal/image.py index 543e7d51c..cb69199ce 100644 --- a/modal/image.py +++ b/modal/image.py @@ -328,10 +328,11 @@ def _assert_no_mount_layers(self): raise InvalidError( textwrap.dedent( """ - An image tried to run a build step after using `image.add_*` to include local files. + An image tried to run a build step after using `image.attach_local_*` to include local files. - Run `image.add_*` commands last in your image build to avoid rebuilding images with every local filechange. - Modal will then mount these files as a thin layer when starting your container, saving build time. + Run `image.attach_local_*` commands last in your image build to avoid rebuilding images with every local + filechange. Modal will then mount these files as a thin layer when starting your container, saving build + time. If you need these files earlier in the build, set `copy=True` to copy the files directly into the image, though this will increase build time. From 1e1bc970abb75f914a9b8d542397df9cf332b3fa Mon Sep 17 00:00:00 2001 From: Elias Freider <elias@modal.com> Date: Fri, 15 Nov 2024 10:49:17 +0100 Subject: [PATCH 27/36] More name changes --- test/image_test.py | 28 ++++++++++++++-------------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/test/image_test.py b/test/image_test.py index 421b10134..2403da17f 100644 --- a/test/image_test.py +++ b/test/image_test.py @@ -1226,15 +1226,15 @@ def hydrate_image(img, client): pass -def test_add_local_lazy_vs_not(client, servicer, set_env_client, supports_on_path): +def test_attach_local_lazy_vs_copy(client, servicer, set_env_client, supports_on_path): deb = Image.debian_slim() - image_with_mount = deb.add_local_python_packages("pkg_a") + image_with_mount = deb.attach_local_python_packages("pkg_a") hydrate_image(image_with_mount, client) assert image_with_mount.object_id == deb.object_id assert len(image_with_mount._mount_layers) == 1 - image_additional_mount = image_with_mount.add_local_python_packages("pkg_b") + image_additional_mount = image_with_mount.attach_local_python_packages("pkg_b") hydrate_image(image_additional_mount, client) assert len(image_additional_mount._mount_layers) == 2 # another mount added to lazy layer assert len(image_with_mount._mount_layers) == 1 # original image should not be affected @@ -1245,12 +1245,12 @@ def test_add_local_lazy_vs_not(client, servicer, set_env_client, supports_on_pat # error about using non-copy add commands before other build steps hydrate_image(image_non_mount, client) - image_with_copy = deb.add_local_python_packages("pkg_a", copy=True) + image_with_copy = deb.attach_local_python_packages("pkg_a", copy=True) hydrate_image(image_with_copy, client) assert len(image_with_copy._mount_layers) == 0 # do the same exact image using copy=True - image_with_copy_and_commands = deb.add_local_python_packages("pkg_a", copy=True).run_commands("echo 'hello'") + image_with_copy_and_commands = deb.attach_local_python_packages("pkg_a", copy=True).run_commands("echo 'hello'") hydrate_image(image_with_copy_and_commands, client) assert len(image_with_copy_and_commands._mount_layers) == 0 @@ -1266,9 +1266,9 @@ def test_add_local_lazy_vs_not(client, servicer, set_env_client, supports_on_pat assert all(fn.startswith("/root/pkg_a/") for fn in copied_files) -def test_add_local_mount_are_attached_to_functions(servicer, client, supports_on_path): +def test_attach_locals_are_attached_to_functions(servicer, client, supports_on_path): deb_slim = Image.debian_slim() - img = deb_slim.add_local_python_packages("pkg_a") + img = deb_slim.attach_local_python_packages("pkg_a") app = App("my-app") control_fun: modal.Function = app.function(serialized=True, image=deb_slim, name="control")( dummy @@ -1283,9 +1283,9 @@ def test_add_local_mount_are_attached_to_functions(servicer, client, supports_on assert added_mounts == {img._mount_layers[0].object_id} -def test_add_local_mount_are_attached_to_classes(servicer, client, supports_on_path, set_env_client): +def test_attach_locals_are_attached_to_classes(servicer, client, supports_on_path, set_env_client): deb_slim = Image.debian_slim() - img = deb_slim.add_local_python_packages("pkg_a") + img = deb_slim.attach_local_python_packages("pkg_a") app = App("my-app") control_fun: modal.Function = app.function(serialized=True, image=deb_slim, name="control")( dummy @@ -1314,9 +1314,9 @@ class A: @skip_windows("servicer sandbox implementation not working on windows") -def test_add_local_mount_are_attached_to_sandboxes(servicer, client, supports_on_path): +def test_attach_locals_are_attached_to_sandboxes(servicer, client, supports_on_path): deb_slim = Image.debian_slim() - img = deb_slim.add_local_python_packages("pkg_a") + img = deb_slim.attach_local_python_packages("pkg_a") app = App("my-app") with app.run(client=client): modal.Sandbox.create(image=img, app=app, client=client) @@ -1333,9 +1333,9 @@ def empty_fun(): pass -def test_add_local_mount_build_function(servicer, client, supports_on_path): +def test_attach_locals_build_function(servicer, client, supports_on_path): deb_slim = Image.debian_slim() - img = deb_slim.add_local_python_packages("pkg_a") + img = deb_slim.attach_local_python_packages("pkg_a") img_with_build_function = img.run_function(empty_fun) with pytest.raises(InvalidError): # build functions could still potentially rewrite mount contents, @@ -1343,7 +1343,7 @@ def test_add_local_mount_build_function(servicer, client, supports_on_path): # TODO(elias): what if someone wants do use an equivalent of `run_function(..., mounts=[...]) ? hydrate_image(img_with_build_function, client) - img_with_copy = deb_slim.add_local_python_packages("pkg_a", copy=True) + img_with_copy = deb_slim.attach_local_python_packages("pkg_a", copy=True) hydrate_image(img_with_copy, client) # this is fine From 286db8277e018e0a2c15b0b0bf7deb65b83941ee Mon Sep 17 00:00:00 2001 From: Elias Freider <elias@modal.com> Date: Tue, 19 Nov 2024 15:33:35 +0100 Subject: [PATCH 28/36] Back to add --- modal/image.py | 4 ++-- test/image_test.py | 18 +++++++++--------- test/watcher_test.py | 2 +- 3 files changed, 12 insertions(+), 12 deletions(-) diff --git a/modal/image.py b/modal/image.py index b5d5edcaf..99f489441 100644 --- a/modal/image.py +++ b/modal/image.py @@ -347,7 +347,7 @@ def _assert_no_mount_layers(self): my_image = ( Image.debian_slim() - .attach_local_python_packages("mypak", copy=True) + .add_local_python_packages("mypak", copy=True) .run_commands("python -m mypak") # this now works! ) """ @@ -624,7 +624,7 @@ def build_dockerfile(version: ImageBuilderVersion) -> DockerfileSpec: context_mount=mount, ) - def attach_local_python_packages(self, *packages: Union[str, Path], copy: bool = False) -> "_Image": + def add_local_python_packages(self, *packages: Union[str, Path], copy: bool = False) -> "_Image": """Attaches local Python packages to the container running the image Packages are added to the /root directory which is on the PYTHONPATH of any diff --git a/test/image_test.py b/test/image_test.py index 2403da17f..2b5db6c90 100644 --- a/test/image_test.py +++ b/test/image_test.py @@ -1228,13 +1228,13 @@ def hydrate_image(img, client): def test_attach_local_lazy_vs_copy(client, servicer, set_env_client, supports_on_path): deb = Image.debian_slim() - image_with_mount = deb.attach_local_python_packages("pkg_a") + image_with_mount = deb.add_local_python_packages("pkg_a") hydrate_image(image_with_mount, client) assert image_with_mount.object_id == deb.object_id assert len(image_with_mount._mount_layers) == 1 - image_additional_mount = image_with_mount.attach_local_python_packages("pkg_b") + image_additional_mount = image_with_mount.add_local_python_packages("pkg_b") hydrate_image(image_additional_mount, client) assert len(image_additional_mount._mount_layers) == 2 # another mount added to lazy layer assert len(image_with_mount._mount_layers) == 1 # original image should not be affected @@ -1245,12 +1245,12 @@ def test_attach_local_lazy_vs_copy(client, servicer, set_env_client, supports_on # error about using non-copy add commands before other build steps hydrate_image(image_non_mount, client) - image_with_copy = deb.attach_local_python_packages("pkg_a", copy=True) + image_with_copy = deb.add_local_python_packages("pkg_a", copy=True) hydrate_image(image_with_copy, client) assert len(image_with_copy._mount_layers) == 0 # do the same exact image using copy=True - image_with_copy_and_commands = deb.attach_local_python_packages("pkg_a", copy=True).run_commands("echo 'hello'") + image_with_copy_and_commands = deb.add_local_python_packages("pkg_a", copy=True).run_commands("echo 'hello'") hydrate_image(image_with_copy_and_commands, client) assert len(image_with_copy_and_commands._mount_layers) == 0 @@ -1268,7 +1268,7 @@ def test_attach_local_lazy_vs_copy(client, servicer, set_env_client, supports_on def test_attach_locals_are_attached_to_functions(servicer, client, supports_on_path): deb_slim = Image.debian_slim() - img = deb_slim.attach_local_python_packages("pkg_a") + img = deb_slim.add_local_python_packages("pkg_a") app = App("my-app") control_fun: modal.Function = app.function(serialized=True, image=deb_slim, name="control")( dummy @@ -1285,7 +1285,7 @@ def test_attach_locals_are_attached_to_functions(servicer, client, supports_on_p def test_attach_locals_are_attached_to_classes(servicer, client, supports_on_path, set_env_client): deb_slim = Image.debian_slim() - img = deb_slim.attach_local_python_packages("pkg_a") + img = deb_slim.add_local_python_packages("pkg_a") app = App("my-app") control_fun: modal.Function = app.function(serialized=True, image=deb_slim, name="control")( dummy @@ -1316,7 +1316,7 @@ class A: @skip_windows("servicer sandbox implementation not working on windows") def test_attach_locals_are_attached_to_sandboxes(servicer, client, supports_on_path): deb_slim = Image.debian_slim() - img = deb_slim.attach_local_python_packages("pkg_a") + img = deb_slim.add_local_python_packages("pkg_a") app = App("my-app") with app.run(client=client): modal.Sandbox.create(image=img, app=app, client=client) @@ -1335,7 +1335,7 @@ def empty_fun(): def test_attach_locals_build_function(servicer, client, supports_on_path): deb_slim = Image.debian_slim() - img = deb_slim.attach_local_python_packages("pkg_a") + img = deb_slim.add_local_python_packages("pkg_a") img_with_build_function = img.run_function(empty_fun) with pytest.raises(InvalidError): # build functions could still potentially rewrite mount contents, @@ -1343,7 +1343,7 @@ def test_attach_locals_build_function(servicer, client, supports_on_path): # TODO(elias): what if someone wants do use an equivalent of `run_function(..., mounts=[...]) ? hydrate_image(img_with_build_function, client) - img_with_copy = deb_slim.attach_local_python_packages("pkg_a", copy=True) + img_with_copy = deb_slim.add_local_python_packages("pkg_a", copy=True) hydrate_image(img_with_copy, client) # this is fine diff --git a/test/watcher_test.py b/test/watcher_test.py index 045d8f62f..f4ea014d8 100644 --- a/test/watcher_test.py +++ b/test/watcher_test.py @@ -112,7 +112,7 @@ def dummy(): def test_add_local_mount_included_in_serve_watchers(servicer, client, supports_on_path, disable_auto_mount): deb_slim = modal.Image.debian_slim() - img = deb_slim.attach_local_python_packages("pkg_a") + img = deb_slim.add_local_python_packages("pkg_a") app = modal.App() @app.function(serialized=True, image=img) From 652fd7dbd86fc208ead188363162a3c859bfbecf Mon Sep 17 00:00:00 2001 From: Elias Freider <elias@modal.com> Date: Tue, 19 Nov 2024 15:38:12 +0100 Subject: [PATCH 29/36] more --- modal/functions.py | 2 +- modal/image.py | 4 ++-- test/image_test.py | 10 +++++----- 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/modal/functions.py b/modal/functions.py index 7b338700a..d4ce57389 100644 --- a/modal/functions.py +++ b/modal/functions.py @@ -634,7 +634,7 @@ def from_args( if proxy: # HACK: remove this once we stop using ssh tunnels for this. if image: - # TODO(elias): this will cause an error if users use prior `.attach_local_*` commands without copy=True + # TODO(elias): this will cause an error if users use prior `.add_local_*` commands without copy=True image = image.apt_install("autossh") function_spec = _FunctionSpec( diff --git a/modal/image.py b/modal/image.py index 0c7663e88..44acf8db1 100644 --- a/modal/image.py +++ b/modal/image.py @@ -334,9 +334,9 @@ def _assert_no_mount_layers(self): raise InvalidError( textwrap.dedent( """ - An image tried to run a build step after using `image.attach_local_*` to include local files. + An image tried to run a build step after using `image.add_local_*` to include local files. - Run `image.attach_local_*` commands last in your image build to avoid rebuilding images with every local + Run `image.add_local_*` commands last in your image build to avoid rebuilding images with every local filechange. Modal will then mount these files as a thin layer when starting your container, saving build time. diff --git a/test/image_test.py b/test/image_test.py index e5f5ab1ec..16cf16b99 100644 --- a/test/image_test.py +++ b/test/image_test.py @@ -1226,7 +1226,7 @@ def hydrate_image(img, client): pass -def test_attach_local_lazy_vs_copy(client, servicer, set_env_client, supports_on_path): +def test_add_local_lazy_vs_copy(client, servicer, set_env_client, supports_on_path): deb = Image.debian_slim() image_with_mount = deb.add_local_python_packages("pkg_a") @@ -1266,7 +1266,7 @@ def test_attach_local_lazy_vs_copy(client, servicer, set_env_client, supports_on assert all(fn.startswith("/root/pkg_a/") for fn in copied_files) -def test_attach_locals_are_attached_to_functions(servicer, client, supports_on_path): +def test_add_locals_are_attached_to_functions(servicer, client, supports_on_path): deb_slim = Image.debian_slim() img = deb_slim.add_local_python_packages("pkg_a") app = App("my-app") @@ -1283,7 +1283,7 @@ def test_attach_locals_are_attached_to_functions(servicer, client, supports_on_p assert added_mounts == {img._mount_layers[0].object_id} -def test_attach_locals_are_attached_to_classes(servicer, client, supports_on_path, set_env_client): +def test_add_locals_are_attached_to_classes(servicer, client, supports_on_path, set_env_client): deb_slim = Image.debian_slim() img = deb_slim.add_local_python_packages("pkg_a") app = App("my-app") @@ -1314,7 +1314,7 @@ class A: @skip_windows("servicer sandbox implementation not working on windows") -def test_attach_locals_are_attached_to_sandboxes(servicer, client, supports_on_path): +def test_add_locals_are_attached_to_sandboxes(servicer, client, supports_on_path): deb_slim = Image.debian_slim() img = deb_slim.add_local_python_packages("pkg_a") app = App("my-app") @@ -1333,7 +1333,7 @@ def empty_fun(): pass -def test_attach_locals_build_function(servicer, client, supports_on_path): +def test_add_locals_build_function(servicer, client, supports_on_path): deb_slim = Image.debian_slim() img = deb_slim.add_local_python_packages("pkg_a") img_with_build_function = img.run_function(empty_fun) From 8645fbba0fc3b71195f05568299fa17a2a453535 Mon Sep 17 00:00:00 2001 From: Elias Freider <elias@modal.com> Date: Thu, 21 Nov 2024 11:28:47 +0100 Subject: [PATCH 30/36] Adjust error message to fewer explicit line breaks --- modal/image.py | 38 ++++++++++++++++---------------------- test/image_test.py | 18 +++++++++--------- test/watcher_test.py | 2 +- 3 files changed, 26 insertions(+), 32 deletions(-) diff --git a/modal/image.py b/modal/image.py index 44acf8db1..139332059 100644 --- a/modal/image.py +++ b/modal/image.py @@ -5,7 +5,6 @@ import re import shlex import sys -import textwrap import typing import warnings from dataclasses import dataclass @@ -332,26 +331,21 @@ def _mount_layers(self) -> typing.Tuple[_Mount]: def _assert_no_mount_layers(self): if self._mount_layers: raise InvalidError( - textwrap.dedent( - """ - An image tried to run a build step after using `image.add_local_*` to include local files. - - Run `image.add_local_*` commands last in your image build to avoid rebuilding images with every local - filechange. Modal will then mount these files as a thin layer when starting your container, saving build - time. - - If you need these files earlier in the build, set `copy=True` to copy the files directly into the image, - though this will increase build time. - - Example: - - my_image = ( - Image.debian_slim() - .add_local_python_packages("mypak", copy=True) - .run_commands("python -m mypak") # this now works! - ) - """ - ) + "An image tried to run a build step after using `image.add_local_*` to include local files.\n" + "\n" + "Run `image.add_local_*` commands last in your image build to avoid rebuilding images with every local " + "file change. Modal will then mount these files as a thin layer when starting your container, " + "saving build time.\n" + "If you need these files earlier in the build, set `copy=True` to copy the files directly into the " + "image, though this will increase build time.\n" + "\n" + "Example:\n" + "\n" + "my_image = (\n" + " Image.debian_slim()\n" + ' .add_local_python_packages("mypak", copy=True)\n' + ' .run_commands("python -m mypak") # this now works!\n' + ")\n" ) @staticmethod @@ -624,7 +618,7 @@ def build_dockerfile(version: ImageBuilderVersion) -> DockerfileSpec: context_mount=mount, ) - def add_local_python_packages(self, *packages: Union[str, Path], copy: bool = False) -> "_Image": + def _add_local_python_packages(self, *packages: Union[str, Path], copy: bool = False) -> "_Image": """Attaches local Python packages to the container running the image Packages are added to the /root directory which is on the PYTHONPATH of any diff --git a/test/image_test.py b/test/image_test.py index 16cf16b99..fa0990d48 100644 --- a/test/image_test.py +++ b/test/image_test.py @@ -1228,13 +1228,13 @@ def hydrate_image(img, client): def test_add_local_lazy_vs_copy(client, servicer, set_env_client, supports_on_path): deb = Image.debian_slim() - image_with_mount = deb.add_local_python_packages("pkg_a") + image_with_mount = deb._add_local_python_packages("pkg_a") hydrate_image(image_with_mount, client) assert image_with_mount.object_id == deb.object_id assert len(image_with_mount._mount_layers) == 1 - image_additional_mount = image_with_mount.add_local_python_packages("pkg_b") + image_additional_mount = image_with_mount._add_local_python_packages("pkg_b") hydrate_image(image_additional_mount, client) assert len(image_additional_mount._mount_layers) == 2 # another mount added to lazy layer assert len(image_with_mount._mount_layers) == 1 # original image should not be affected @@ -1245,12 +1245,12 @@ def test_add_local_lazy_vs_copy(client, servicer, set_env_client, supports_on_pa # error about using non-copy add commands before other build steps hydrate_image(image_non_mount, client) - image_with_copy = deb.add_local_python_packages("pkg_a", copy=True) + image_with_copy = deb._add_local_python_packages("pkg_a", copy=True) hydrate_image(image_with_copy, client) assert len(image_with_copy._mount_layers) == 0 # do the same exact image using copy=True - image_with_copy_and_commands = deb.add_local_python_packages("pkg_a", copy=True).run_commands("echo 'hello'") + image_with_copy_and_commands = deb._add_local_python_packages("pkg_a", copy=True).run_commands("echo 'hello'") hydrate_image(image_with_copy_and_commands, client) assert len(image_with_copy_and_commands._mount_layers) == 0 @@ -1268,7 +1268,7 @@ def test_add_local_lazy_vs_copy(client, servicer, set_env_client, supports_on_pa def test_add_locals_are_attached_to_functions(servicer, client, supports_on_path): deb_slim = Image.debian_slim() - img = deb_slim.add_local_python_packages("pkg_a") + img = deb_slim._add_local_python_packages("pkg_a") app = App("my-app") control_fun: modal.Function = app.function(serialized=True, image=deb_slim, name="control")( dummy @@ -1285,7 +1285,7 @@ def test_add_locals_are_attached_to_functions(servicer, client, supports_on_path def test_add_locals_are_attached_to_classes(servicer, client, supports_on_path, set_env_client): deb_slim = Image.debian_slim() - img = deb_slim.add_local_python_packages("pkg_a") + img = deb_slim._add_local_python_packages("pkg_a") app = App("my-app") control_fun: modal.Function = app.function(serialized=True, image=deb_slim, name="control")( dummy @@ -1316,7 +1316,7 @@ class A: @skip_windows("servicer sandbox implementation not working on windows") def test_add_locals_are_attached_to_sandboxes(servicer, client, supports_on_path): deb_slim = Image.debian_slim() - img = deb_slim.add_local_python_packages("pkg_a") + img = deb_slim._add_local_python_packages("pkg_a") app = App("my-app") with app.run(client=client): modal.Sandbox.create(image=img, app=app, client=client) @@ -1335,7 +1335,7 @@ def empty_fun(): def test_add_locals_build_function(servicer, client, supports_on_path): deb_slim = Image.debian_slim() - img = deb_slim.add_local_python_packages("pkg_a") + img = deb_slim._add_local_python_packages("pkg_a") img_with_build_function = img.run_function(empty_fun) with pytest.raises(InvalidError): # build functions could still potentially rewrite mount contents, @@ -1343,7 +1343,7 @@ def test_add_locals_build_function(servicer, client, supports_on_path): # TODO(elias): what if someone wants do use an equivalent of `run_function(..., mounts=[...]) ? hydrate_image(img_with_build_function, client) - img_with_copy = deb_slim.add_local_python_packages("pkg_a", copy=True) + img_with_copy = deb_slim._add_local_python_packages("pkg_a", copy=True) hydrate_image(img_with_copy, client) # this is fine diff --git a/test/watcher_test.py b/test/watcher_test.py index f4ea014d8..1386a1afc 100644 --- a/test/watcher_test.py +++ b/test/watcher_test.py @@ -112,7 +112,7 @@ def dummy(): def test_add_local_mount_included_in_serve_watchers(servicer, client, supports_on_path, disable_auto_mount): deb_slim = modal.Image.debian_slim() - img = deb_slim.add_local_python_packages("pkg_a") + img = deb_slim._add_local_python_packages("pkg_a") app = modal.App() @app.function(serialized=True, image=img) From 222dc25df517899f1340402ab10e14f028b9c89a Mon Sep 17 00:00:00 2001 From: Elias Freider <elias@modal.com> Date: Thu, 21 Nov 2024 11:30:53 +0100 Subject: [PATCH 31/36] change repr --- modal/image.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modal/image.py b/modal/image.py index 139332059..ec4137bfc 100644 --- a/modal/image.py +++ b/modal/image.py @@ -313,7 +313,7 @@ async def _load(self2: "_Image", resolver: Resolver, existing_object_id: Optiona self2._mounts = tuple(base_image._mounts) + (mount,) self2._used_local_mounts = base_image._used_local_mounts | ({mount} if mount.is_local() else set()) - return _Image._from_loader(_load, "ImageWithMounts()", deps=lambda: [base_image, mount]) + return _Image._from_loader(_load, "Image(local files)", deps=lambda: [base_image, mount]) @property def _mount_layers(self) -> typing.Tuple[_Mount]: From 67ed484b954b2071e50ddf9bfc8c7d56192e832e Mon Sep 17 00:00:00 2001 From: Elias Freider <elias@modal.com> Date: Thu, 21 Nov 2024 12:48:30 +0100 Subject: [PATCH 32/36] docstring --- modal/image.py | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/modal/image.py b/modal/image.py index ec4137bfc..80f5612db 100644 --- a/modal/image.py +++ b/modal/image.py @@ -619,18 +619,22 @@ def build_dockerfile(version: ImageBuilderVersion) -> DockerfileSpec: ) def _add_local_python_packages(self, *packages: Union[str, Path], copy: bool = False) -> "_Image": - """Attaches local Python packages to the container running the image + """Adds all files from the specified Python packages to containers running the image Packages are added to the /root directory which is on the PYTHONPATH of any executed Modal functions. - By default (copy=False) the packages are layered on top of the image when - the container starts up and not built as an image layer. + By default (copy=False) the files are added to your containers when they + start up and not built into the actual image, which speeds up deployment. - Set copy=True to force the packages to be added as an image layer instead. - This can be slower since it requires a rebuild of the image whenever the - required package files change, but it allows you to run additional build - steps after this operation. + Set copy=True to force the files to be added as an image layer instead. + This can slow down deployment since it requires a rebuild of the image + and any subsequent build steps whenever the included files change, but + it allows you to run additional build steps after this one. + + Note that this excludes all .-prefixed sub-directories or files and all + .pyc/__pycache__ files. To add full directories with finer control use + `.add_local_dir()` instead. """ mount = _Mount.from_local_python_packages(*packages) return self._add_mount_layer_or_copy(mount, copy=copy) From c0d82369f99629144d8d1a55244044b22015306a Mon Sep 17 00:00:00 2001 From: Elias Freider <elias@modal.com> Date: Thu, 21 Nov 2024 12:58:52 +0100 Subject: [PATCH 33/36] Docstring --- modal/image.py | 23 +++++++++++------------ 1 file changed, 11 insertions(+), 12 deletions(-) diff --git a/modal/image.py b/modal/image.py index 80f5612db..2f5b15719 100644 --- a/modal/image.py +++ b/modal/image.py @@ -619,22 +619,21 @@ def build_dockerfile(version: ImageBuilderVersion) -> DockerfileSpec: ) def _add_local_python_packages(self, *packages: Union[str, Path], copy: bool = False) -> "_Image": - """Adds all files from the specified Python packages to containers running the image + """Adds Python Package Files to Containers - Packages are added to the /root directory which is on the PYTHONPATH of any - executed Modal functions. + Adds all files from the specified Python packages to containers running the image. - By default (copy=False) the files are added to your containers when they - start up and not built into the actual image, which speeds up deployment. + Packages are added to the `/root` directory, which is on the `PYTHONPATH` of any executed Modal functions. - Set copy=True to force the files to be added as an image layer instead. - This can slow down deployment since it requires a rebuild of the image - and any subsequent build steps whenever the included files change, but - it allows you to run additional build steps after this one. + By default (`copy=False`), the files are added to containers on startup and are not built into the actual image, + which speeds up deployment. - Note that this excludes all .-prefixed sub-directories or files and all - .pyc/__pycache__ files. To add full directories with finer control use - `.add_local_dir()` instead. + Set `copy=True` to copy the files into an image layer at build time instead. This can slow down iteration since + it requires a rebuild of the image and any subsequent build steps whenever the included files change, but it is + required if you want to run additional build steps after this one. + + **Note:** This excludes all dot-prefixed subdirectories or files and all `.pyc`/`__pycache__` files. + To add full directories with finer control, use `.add_local_dir()` instead. """ mount = _Mount.from_local_python_packages(*packages) return self._add_mount_layer_or_copy(mount, copy=copy) From 11452a8de87df5fc7158948c6b04bd4b19b43e0e Mon Sep 17 00:00:00 2001 From: Elias Freider <elias@modal.com> Date: Thu, 21 Nov 2024 13:04:31 +0100 Subject: [PATCH 34/36] Renames and more copy changes --- modal/app.py | 2 +- modal/functions.py | 8 ++++---- modal/image.py | 27 +++++++++++++++------------ 3 files changed, 20 insertions(+), 17 deletions(-) diff --git a/modal/app.py b/modal/app.py index 125540510..331f2d954 100644 --- a/modal/app.py +++ b/modal/app.py @@ -488,7 +488,7 @@ def _get_watch_mounts(self): *self._mounts, ] for function in self.registered_functions.values(): - all_mounts.extend(function._used_local_mounts) + all_mounts.extend(function._serve_mounts) return [m for m in all_mounts if m.is_local()] diff --git a/modal/functions.py b/modal/functions.py index d4ce57389..4f156c711 100644 --- a/modal/functions.py +++ b/modal/functions.py @@ -305,7 +305,7 @@ class _Function(typing.Generic[P, ReturnType, OriginalReturnType], _Object, type # TODO: more type annotations _info: Optional[FunctionInfo] - _used_local_mounts: typing.FrozenSet[_Mount] # set at load time, only by loader + _serve_mounts: typing.FrozenSet[_Mount] # set at load time, only by loader _app: Optional["modal.app._App"] = None _obj: Optional["modal.cls._Obj"] = None # only set for InstanceServiceFunctions and bound instance methods _web_url: Optional[str] @@ -994,8 +994,8 @@ async def _load(self: _Function, resolver: Resolver, existing_object_id: Optiona raise function_creation_status.set_response(response) local_mounts = set(m for m in all_mounts if m.is_local()) # needed for modal.serve file watching - local_mounts |= image._used_local_mounts - obj._used_local_mounts = frozenset(local_mounts) + local_mounts |= image._serve_mounts + obj._serve_mounts = frozenset(local_mounts) self._hydrate_function_and_method_functions(response.function_id, resolver.client, response.handle_metadata) rep = f"Function({tag})" @@ -1252,7 +1252,7 @@ def _initialize_from_empty(self): self._function_name = None self._info = None self._use_function_id = "" - self._used_local_mounts = frozenset() + self._serve_mounts = frozenset() def _hydrate_metadata(self, metadata: Optional[Message]): # Overridden concrete implementation of base class method diff --git a/modal/image.py b/modal/image.py index 2f5b15719..dfd23e5fb 100644 --- a/modal/image.py +++ b/modal/image.py @@ -273,22 +273,24 @@ class _Image(_Object, type_prefix="im"): force_build: bool inside_exceptions: List[Exception] - _used_local_mounts: typing.FrozenSet[_Mount] # used for mounts watching - _mounts: Sequence[_Mount] # added as mounts on any container referencing the Image, see `def _mount_layers` + _serve_mounts: typing.FrozenSet[_Mount] # used for mounts watching in `modal serve` + _deferred_mounts: Sequence[ + _Mount + ] # added as mounts on any container referencing the Image, see `def _mount_layers` _metadata: Optional[api_pb2.ImageMetadata] = None # set on hydration, private for now def _initialize_from_empty(self): self.inside_exceptions = [] - self._used_local_mounts = frozenset() - self._mounts = () + self._serve_mounts = frozenset() + self._deferred_mounts = () self.force_build = False def _initialize_from_other(self, other: "_Image"): # used by .clone() self.inside_exceptions = other.inside_exceptions self.force_build = other.force_build - self._used_local_mounts = other._used_local_mounts - self._mounts = other._mounts + self._serve_mounts = other._serve_mounts + self._deferred_mounts = other._deferred_mounts def _hydrate_metadata(self, message: Optional[Message]): env_image_id = config.get("image_id") # set as an env var in containers @@ -310,8 +312,8 @@ def _add_mount_layer_or_copy(self, mount: _Mount, copy: bool = False): async def _load(self2: "_Image", resolver: Resolver, existing_object_id: Optional[str]): self2._hydrate_from_other(base_image) # same image id as base image as long as it's lazy - self2._mounts = tuple(base_image._mounts) + (mount,) - self2._used_local_mounts = base_image._used_local_mounts | ({mount} if mount.is_local() else set()) + self2._deferred_mounts = tuple(base_image._deferred_mounts) + (mount,) + self2._serve_mounts = base_image._serve_mounts | ({mount} if mount.is_local() else set()) return _Image._from_loader(_load, "Image(local files)", deps=lambda: [base_image, mount]) @@ -326,7 +328,7 @@ def _mount_layers(self) -> typing.Tuple[_Mount]: When the image is used as a base image for a new layer (that is not itself a mount layer) these mounts need to first be inserted as a copy operation (.copy_mount) into the image. """ - return self._mounts + return self._deferred_mounts def _assert_no_mount_layers(self): if self._mount_layers: @@ -547,10 +549,10 @@ async def join(): self._hydrate(image_id, resolver.client, result_response.metadata) local_mounts = set() for base in base_images.values(): - local_mounts |= base._used_local_mounts + local_mounts |= base._serve_mounts if context_mount and context_mount.is_local(): local_mounts.add(context_mount) - self._used_local_mounts = frozenset(local_mounts) + self._serve_mounts = frozenset(local_mounts) rep = f"Image({dockerfile_function})" obj = _Image._from_loader(_load, rep, deps=_deps) @@ -623,7 +625,8 @@ def _add_local_python_packages(self, *packages: Union[str, Path], copy: bool = F Adds all files from the specified Python packages to containers running the image. - Packages are added to the `/root` directory, which is on the `PYTHONPATH` of any executed Modal functions. + Packages are added to the `/root` directory of containers, which is on the `PYTHONPATH` + of any executed Modal functions. By default (`copy=False`), the files are added to containers on startup and are not built into the actual image, which speeds up deployment. From 48789458280c244c0b4e6c76a334bbd562eca8ca Mon Sep 17 00:00:00 2001 From: Elias Freider <elias@modal.com> Date: Thu, 21 Nov 2024 13:06:15 +0100 Subject: [PATCH 35/36] More copy --- modal/image.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/modal/image.py b/modal/image.py index dfd23e5fb..8742dd9fb 100644 --- a/modal/image.py +++ b/modal/image.py @@ -621,18 +621,18 @@ def build_dockerfile(version: ImageBuilderVersion) -> DockerfileSpec: ) def _add_local_python_packages(self, *packages: Union[str, Path], copy: bool = False) -> "_Image": - """Adds Python Package Files to Containers + """Adds Python package files to containers - Adds all files from the specified Python packages to containers running the image. + Adds all files from the specified Python packages to containers running the Image. Packages are added to the `/root` directory of containers, which is on the `PYTHONPATH` - of any executed Modal functions. + of any executed Modal Functions. - By default (`copy=False`), the files are added to containers on startup and are not built into the actual image, + By default (`copy=False`), the files are added to containers on startup and are not built into the actual Image, which speeds up deployment. - Set `copy=True` to copy the files into an image layer at build time instead. This can slow down iteration since - it requires a rebuild of the image and any subsequent build steps whenever the included files change, but it is + Set `copy=True` to copy the files into an Image layer at build time instead. This can slow down iteration since + it requires a rebuild of the Image and any subsequent build steps whenever the included files change, but it is required if you want to run additional build steps after this one. **Note:** This excludes all dot-prefixed subdirectories or files and all `.pyc`/`__pycache__` files. From 94f98430b18e739ed40fcf3c1f94fa0998d4c2f2 Mon Sep 17 00:00:00 2001 From: Elias Freider <elias@modal.com> Date: Thu, 21 Nov 2024 13:08:34 +0100 Subject: [PATCH 36/36] More copy --- modal/image.py | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/modal/image.py b/modal/image.py index 8742dd9fb..32ab4512d 100644 --- a/modal/image.py +++ b/modal/image.py @@ -336,10 +336,9 @@ def _assert_no_mount_layers(self): "An image tried to run a build step after using `image.add_local_*` to include local files.\n" "\n" "Run `image.add_local_*` commands last in your image build to avoid rebuilding images with every local " - "file change. Modal will then mount these files as a thin layer when starting your container, " - "saving build time.\n" - "If you need these files earlier in the build, set `copy=True` to copy the files directly into the " - "image, though this will increase build time.\n" + "file change. Modal will then add these files to containers on startup instead, saving build time.\n" + "If you need to run other build steps after adding local files, set `copy=True` to copy the files" + "directly into the image, at the expense of some added build time.\n" "\n" "Example:\n" "\n"