Adds warnings if automounts seem to be required

modal/_functions.py

@@ -65,7 +65,7 @@
 )
 from .gpu import GPU_T, parse_gpu_config
 from .image import _Image
-from .mount import _get_client_mount, _Mount, get_auto_mounts
+from .mount import _get_client_mount, _Mount, get_sys_modules_mounts
 from .network_file_system import _NetworkFileSystem, network_file_system_mount_protos
 from .output import _get_output_manager
 from .parallel_map import (
@@ -434,7 +434,7 @@ def _bind_method(
         return fun
 
     @staticmethod
-    def from_args(
+    def from_local(
         info: FunctionInfo,
         app,
         image: _Image,
@@ -501,18 +501,41 @@ def from_args(
             if include_source_mode != IncludeSourceMode.INCLUDE_NOTHING:
                 entrypoint_mounts = info.get_entrypoint_mount()
             else:
-                entrypoint_mounts = []
+                entrypoint_mounts = {}
 
             all_mounts = [
                 _get_client_mount(),
                 *explicit_mounts,
-                *entrypoint_mounts,
+                *entrypoint_mounts.values(),
             ]
 
             if include_source_mode is IncludeSourceMode.INCLUDE_FIRST_PARTY:
-                # TODO(elias): if using INCLUDE_FIRST_PARTY *and* mounts are added that haven't already been
-                #  added to the image via add_local_python_source
-                all_mounts += get_auto_mounts()
+                auto_mounts = get_sys_modules_mounts()
+                # don't need to add entrypoint modules to automounts:
+                for entrypoint_module in entrypoint_mounts:
+                    auto_mounts.pop(entrypoint_module, None)
+
+                warn_missing_modules = set(auto_mounts.keys()) - image._added_python_source_set
+
+                if warn_missing_modules:
+                    python_stringified_modules = ", ".join(f'"{mod}"' for mod in sorted(warn_missing_modules))
+                    deprecation_warning(
+                        (2024, 1, 28),
+                        (
+                            "Automatic adding of local python source will be deprecated in the future.\n"
+                            f"Make sure you have explicitly added the source for the following modules to the image "
+                            f"used by `{info.function_name}`:\n"
+                        )
+                        + ", ".join(sorted(warn_missing_modules))
+                        + "\n\n"
+                        + (
+                            "This can be using `Image.add_local_python_source`, e.g.:\n"
+                            f"image_with_source = Image.debian_slim().add_local_python_source"
+                            f"({python_stringified_modules})"
+                        ),
+                        pending=True,
+                    )
+                all_mounts += auto_mounts.values()
         else:
             # skip any mount introspection/logic inside containers, since the function
             # should already be hydrated
@@ -561,7 +584,7 @@ def from_args(
             for k, pf in build_functions:
                 build_function = pf.raw_f
                 snapshot_info = FunctionInfo(build_function, user_cls=info.user_cls)
-                snapshot_function = _Function.from_args(
+                snapshot_function = _Function.from_local(
                     snapshot_info,
                     app=None,
                     image=image,

modal/_utils/function_utils.py

@@ -308,7 +308,7 @@ def class_parameter_info(self) -> api_pb2.ClassParameterInfo:
             format=api_pb2.ClassParameterInfo.PARAM_SERIALIZATION_FORMAT_PROTO, schema=modal_parameters
         )
 
-    def get_entrypoint_mount(self) -> list[_Mount]:
+    def get_entrypoint_mount(self) -> dict[str, _Mount]:
         """
         Includes:
         * Implicit mount of the function itself (the module or package that the function is part of)
@@ -322,7 +322,7 @@ def get_entrypoint_mount(self) -> list[_Mount]:
         """
         if self.is_serialized():
             # Don't auto-mount anything for serialized functions (including notebooks)
-            return []
+            return {}
 
         # make sure the function's own entrypoint is included:
         if self._type == FunctionInfoType.PACKAGE:
@@ -331,21 +331,23 @@ def get_entrypoint_mount(self) -> list[_Mount]:
             #  includes non-.py files, since we'll want to migrate to .py-only
             #  soon to get it consistent with the `add_local_python_source()`
             #  defaults.
-            return [_Mount._from_local_python_packages(top_level_package)]
+            return {top_level_package: _Mount._from_local_python_packages(top_level_package)}
         elif self._type == FunctionInfoType.FILE:
-            remote_path = ROOT_DIR / Path(self._file).name
+            # TODO: inspect if this file is already included as part of
+            #  a package mount, and skip it + reference that package
+            #  instead if that's the case. This avoids possible module
+            #  duplication bugs
+            module_file = Path(self._file)
+            container_module_name = module_file.stem
+            remote_path = ROOT_DIR / module_file.name
             if not _is_modal_path(remote_path):
-                # TODO: inspect if this file is already included as part of
-                #  a package mount, and skip it + reference that package
-                #  instead if that's the case. This avoids possible module
-                #  duplication bugs
-                return [
-                    _Mount._from_local_file(
+                return {
+                    container_module_name: _Mount._from_local_file(
                         self._file,
                         remote_path=remote_path,
                     )
-                ]
-        return []  # this should never be reached...
+                }
+        return {}  # this should never be reached...
 
     def get_tag(self):
         return self.function_name

modal/app.py

@@ -724,7 +724,7 @@ def f(self, x):
                     raise InvalidError("`region` and `_experimental_scheduler_placement` cannot be used together")
                 scheduler_placement = SchedulerPlacement(region=region)
 
-            function = _Function.from_args(
+            function = _Function.from_local(
                 info,
                 app=self,
                 image=image,
@@ -859,7 +859,7 @@ def wrapper(user_cls: CLS_T) -> CLS_T:
 
             info = FunctionInfo(None, serialized=serialized, user_cls=user_cls)
 
-            cls_func = _Function.from_args(
+            cls_func = _Function.from_local(
                 info,
                 app=self,
                 image=image or self._get_default_image(),

modal/image.py

@@ -402,12 +402,14 @@ class _Image(_Object, type_prefix="im"):
     _deferred_mounts: Sequence[
         _Mount
     ]  # added as mounts on any container referencing the Image, see `def _mount_layers`
+    _added_python_source_set: frozenset[str]  # used to warn about missing mounts during auto-mount deprecation
     _metadata: Optional[api_pb2.ImageMetadata] = None  # set on hydration, private for now
 
     def _initialize_from_empty(self):
         self.inside_exceptions = []
         self._serve_mounts = frozenset()
         self._deferred_mounts = ()
+        self._added_python_source_set = frozenset()
         self.force_build = False
 
     def _initialize_from_other(self, other: "_Image"):
@@ -416,6 +418,7 @@ def _initialize_from_other(self, other: "_Image"):
         self.force_build = other.force_build
         self._serve_mounts = other._serve_mounts
         self._deferred_mounts = other._deferred_mounts
+        self._added_python_source_set = other._added_python_source_set
 
     def _hydrate_metadata(self, metadata: Optional[Message]):
         env_image_id = config.get("image_id")  # set as an env var in containers
@@ -657,9 +660,18 @@ async def _load(self: _Image, resolver: Resolver, existing_object_id: Optional[s
             self._serve_mounts = frozenset(local_mounts)
 
         rep = f"Image({dockerfile_function})"
-        obj = _Image._from_loader(_load, rep, deps=_deps)
-        obj.force_build = force_build
-        return obj
+        img = _Image._from_loader(_load, rep, deps=_deps)
+        img.force_build = force_build
+        return img
+
+    def _extend(self, **kwargs) -> "_Image":
+        """Internal use only - helper method to create a new Image with self as base
+
+        Transfers required static attributes to the new layer as expected.
+        """
+        img = _Image._from_args(base_images={"base": self}, **kwargs)
+        img._added_python_source_set = self._added_python_source_set
+        return img
 
     def copy_mount(self, mount: _Mount, remote_path: Union[str, Path] = ".") -> "_Image":
         """
@@ -686,8 +698,7 @@ def build_dockerfile(version: ImageBuilderVersion) -> DockerfileSpec:
             commands = ["FROM base", f"COPY . {remote_path}"]  # copy everything from the supplied mount
             return DockerfileSpec(commands=commands, context_files={})
 
-        return _Image._from_args(
-            base_images={"base": self},
+        return self._extend(
             dockerfile_function=build_dockerfile,
             context_mount_function=lambda: mount,
         )
@@ -801,8 +812,7 @@ def copy_local_file(self, local_path: Union[str, Path], remote_path: Union[str,
         def build_dockerfile(version: ImageBuilderVersion) -> DockerfileSpec:
             return DockerfileSpec(commands=["FROM base", f"COPY {basename} {remote_path}"], context_files={})
 
-        return _Image._from_args(
-            base_images={"base": self},
+        return self._extend(
             dockerfile_function=build_dockerfile,
             context_mount_function=lambda: _Mount._from_local_file(local_path, remote_path=f"/{basename}"),
         )
@@ -843,7 +853,9 @@ def add_local_python_source(
         ```
         """
         mount = _Mount._from_local_python_packages(*modules, ignore=ignore)
-        return self._add_mount_layer_or_copy(mount, copy=copy)
+        img = self._add_mount_layer_or_copy(mount, copy=copy)
+        img._added_python_source_set |= set(modules)
+        return img
 
     def copy_local_dir(
         self,
@@ -908,8 +920,7 @@ def copy_local_dir(
         def build_dockerfile(version: ImageBuilderVersion) -> DockerfileSpec:
             return DockerfileSpec(commands=["FROM base", f"COPY . {remote_path}"], context_files={})
 
-        return _Image._from_args(
-            base_images={"base": self},
+        return self._extend(
             dockerfile_function=build_dockerfile,
             context_mount_function=lambda: _Mount._add_local_dir(
                 Path(local_path), PurePosixPath("/"), ignore=_ignore_fn(ignore)
@@ -988,8 +999,7 @@ def build_dockerfile(version: ImageBuilderVersion) -> DockerfileSpec:
             return DockerfileSpec(commands=commands, context_files={})
 
         gpu_config = parse_gpu_config(gpu)
-        return _Image._from_args(
-            base_images={"base": self},
+        return self._extend(
             dockerfile_function=build_dockerfile,
             force_build=self.force_build or force_build,
             gpu_config=gpu_config,
@@ -1089,8 +1099,7 @@ def build_dockerfile(version: ImageBuilderVersion) -> DockerfileSpec:
 
         gpu_config = parse_gpu_config(gpu)
 
-        return _Image._from_args(
-            base_images={"base": self},
+        return self._extend(
             dockerfile_function=build_dockerfile,
             secrets=secrets,
             gpu_config=gpu_config,
@@ -1129,8 +1138,7 @@ def build_dockerfile(version: ImageBuilderVersion) -> DockerfileSpec:
                 commands = [cmd.strip() for cmd in commands]
             return DockerfileSpec(commands=commands, context_files=context_files)
 
-        return _Image._from_args(
-            base_images={"base": self},
+        return self._extend(
             dockerfile_function=build_dockerfile,
             force_build=self.force_build or force_build,
             gpu_config=parse_gpu_config(gpu),
@@ -1191,8 +1199,7 @@ def build_dockerfile(version: ImageBuilderVersion) -> DockerfileSpec:
 
             return DockerfileSpec(commands=commands, context_files={})
 
-        return _Image._from_args(
-            base_images={"base": self},
+        return self._extend(
             dockerfile_function=build_dockerfile,
             force_build=self.force_build or force_build,
             secrets=secrets,
@@ -1272,8 +1279,7 @@ def build_dockerfile(version: ImageBuilderVersion) -> DockerfileSpec:
             ]
             return DockerfileSpec(commands=commands, context_files=context_files)
 
-        return _Image._from_args(
-            base_images={"base": self},
+        return self._extend(
             dockerfile_function=build_dockerfile,
             force_build=self.force_build or force_build,
             secrets=secrets,
@@ -1346,8 +1352,7 @@ def dockerfile_commands(
         def build_dockerfile(version: ImageBuilderVersion) -> DockerfileSpec:
             return DockerfileSpec(commands=["FROM base", *cmds], context_files=context_files)
 
-        return _Image._from_args(
-            base_images={"base": self},
+        return self._extend(
             dockerfile_function=build_dockerfile,
             secrets=secrets,
             gpu_config=parse_gpu_config(gpu),
@@ -1394,8 +1399,7 @@ def run_commands(
         def build_dockerfile(version: ImageBuilderVersion) -> DockerfileSpec:
             return DockerfileSpec(commands=["FROM base"] + [f"RUN {cmd}" for cmd in cmds], context_files={})
 
-        return _Image._from_args(
-            base_images={"base": self},
+        return self._extend(
             dockerfile_function=build_dockerfile,
             secrets=secrets,
             gpu_config=parse_gpu_config(gpu),
@@ -1505,8 +1509,7 @@ def build_dockerfile(version: ImageBuilderVersion) -> DockerfileSpec:
             context_files = {} if spec_file is None else {remote_spec_file: os.path.expanduser(spec_file)}
             return DockerfileSpec(commands=commands, context_files=context_files)
 
-        return _Image._from_args(
-            base_images={"base": self},
+        return self._extend(
             dockerfile_function=build_dockerfile,
             force_build=self.force_build or force_build,
             secrets=secrets,
@@ -1820,8 +1823,7 @@ def build_dockerfile_python(version: ImageBuilderVersion) -> DockerfileSpec:
             context_files = {CONTAINER_REQUIREMENTS_PATH: requirements_path}
             return DockerfileSpec(commands=commands, context_files=context_files)
 
-        return _Image._from_args(
-            base_images={"base": base_image},
+        return base_image._extend(
             dockerfile_function=build_dockerfile_python,
             context_mount_function=add_python_mount,
             force_build=force_build,
@@ -1888,8 +1890,7 @@ def build_dockerfile(version: ImageBuilderVersion) -> DockerfileSpec:
             ]
             return DockerfileSpec(commands=commands, context_files={})
 
-        return _Image._from_args(
-            base_images={"base": self},
+        return self._extend(
             dockerfile_function=build_dockerfile,
             force_build=self.force_build or force_build,
             gpu_config=parse_gpu_config(gpu),
@@ -1954,7 +1955,7 @@ def my_build_function():
 
         info = FunctionInfo(raw_f)
 
-        function = _Function.from_args(
+        function = _Function.from_local(
             info,
             app=None,
             image=self,  # type: ignore[reportArgumentType]  # TODO: probably conflict with type stub?
@@ -1980,8 +1981,7 @@ def my_build_function():
             build_function_input = api_pb2.FunctionInput(args=args_serialized, data_format=api_pb2.DATA_FORMAT_PICKLE)
         else:
             build_function_input = None
-        return _Image._from_args(
-            base_images={"base": self},
+        return self._extend(
             build_function=function,
             build_function_input=build_function_input,
             force_build=self.force_build or force_build,
@@ -2004,8 +2004,7 @@ def build_dockerfile(version: ImageBuilderVersion) -> DockerfileSpec:
             commands = ["FROM base"] + [f"ENV {key}={shlex.quote(val)}" for (key, val) in vars.items()]
             return DockerfileSpec(commands=commands, context_files={})
 
-        return _Image._from_args(
-            base_images={"base": self},
+        return self._extend(
             dockerfile_function=build_dockerfile,
         )
 
@@ -2028,8 +2027,7 @@ def build_dockerfile(version: ImageBuilderVersion) -> DockerfileSpec:
             commands = ["FROM base", f"WORKDIR {shlex.quote(str(path))}"]
             return DockerfileSpec(commands=commands, context_files={})
 
-        return _Image._from_args(
-            base_images={"base": self},
+        return self._extend(
             dockerfile_function=build_dockerfile,
         )
 

modal/mount.py

@@ -821,14 +821,14 @@ def _is_modal_path(remote_path: PurePosixPath):
     return False
 
 
-def get_auto_mounts() -> list[_Mount]:
+def get_sys_modules_mounts() -> dict[str, _Mount]:
     """mdmd:hidden
 
     Auto-mount local modules that have been imported in global scope.
     This may or may not include the "entrypoint" of the function as well, depending on how modal is invoked
     Note: sys.modules may change during the iteration
     """
-    auto_mounts = []
+    auto_mounts = {}
     top_level_modules = []
     skip_prefixes = set()
     for name, module in sorted(sys.modules.items(), key=lambda kv: len(kv[0])):
@@ -858,6 +858,6 @@ def get_auto_mounts() -> list[_Mount]:
                 # skip any module that has paths in SYS_PREFIXES, or would overwrite the modal Package in the container
                 break
         else:
-            auto_mounts.append(potential_mount)
+            auto_mounts[module_name] = potential_mount
 
     return auto_mounts