From b9217cc46fcf5752370ca48851abccce7c144c2e Mon Sep 17 00:00:00 2001 From: Michael Waskom Date: Fri, 10 Jan 2025 22:30:08 +0000 Subject: [PATCH 1/2] Fix bug with add_python='3.9' --- modal/image.py | 3 ++- test/conftest.py | 9 ++++++++- test/image_test.py | 23 +++++++++++++++++++++++ 3 files changed, 33 insertions(+), 2 deletions(-) diff --git a/modal/image.py b/modal/image.py index 1b0666401..d53d44d7f 100644 --- a/modal/image.py +++ b/modal/image.py @@ -1506,7 +1506,8 @@ def _registry_setup_commands( "COPY /python/. /usr/local", "ENV TERMINFO_DIRS=/etc/terminfo:/lib/terminfo:/usr/share/terminfo:/usr/lib/terminfo", ] - if add_python < "3.13": + python_minor = add_python.split(".")[1] + if int(python_minor) < 13: # Previous versions did not include the `python` binary, but later ones do. # (The important factor is not the Python version itself, but the standalone dist version.) # We insert the command in the list at the position it was previously always added diff --git a/test/conftest.py b/test/conftest.py index bf50168ba..c9d46c595 100644 --- a/test/conftest.py +++ b/test/conftest.py @@ -47,7 +47,7 @@ from modal.cls import _Cls from modal.functions import _Function from modal.image import ImageBuilderVersion -from modal.mount import client_mount_name +from modal.mount import PYTHON_STANDALONE_VERSIONS, client_mount_name, python_standalone_mount_name from modal_proto import api_grpc, api_pb2 @@ -233,6 +233,13 @@ def __init__(self, blob_host, blobs, credentials): self.default_published_client_mount = "mo-123" self.deployed_mounts = { (client_mount_name(), api_pb2.DEPLOYMENT_NAMESPACE_GLOBAL): self.default_published_client_mount, + **{ + ( + python_standalone_mount_name(version), + api_pb2.DEPLOYMENT_NAMESPACE_GLOBAL, + ): f"mo-{version.replace('.', '')}" + for version in PYTHON_STANDALONE_VERSIONS + }, } self.deployed_nfss = {} diff --git a/test/image_test.py b/test/image_test.py index 3c84702d8..9c299a231 100644 --- a/test/image_test.py +++ b/test/image_test.py @@ -323,6 +323,29 @@ def test_debian_slim_apt_install(builder_version, servicer, client): assert any("pip install numpy" in cmd for cmd in layers[2].dockerfile_commands) +def test_from_registry_add_python(builder_version, servicer, client): + app = App(image=Image.from_registry("ubuntu", add_python="3.9")) + app.function()(dummy) + + with app.run(client=client): + layers = get_image_layers(app.image.object_id, servicer) + commands = layers[0].dockerfile_commands + print(commands) + assert any("COPY /python/. /usr/local" in cmd for cmd in commands) + assert any("ln -s /usr/local/bin/python3" in cmd for cmd in commands) + + if builder_version >= "2024.10": + app = App(image=Image.from_registry("ubuntu", add_python="3.13")) + app.function()(dummy) + + with app.run(client=client): + layers = get_image_layers(app.image.object_id, servicer) + commands = layers[0].dockerfile_commands + print(commands) + assert any("COPY /python/. /usr/local" in cmd for cmd in commands) + assert not any("ln -s /usr/local/bin/python3" in cmd for cmd in commands) + + def test_image_pip_install_pyproject(builder_version, servicer, client): pyproject_toml = os.path.join(os.path.dirname(__file__), "supports/test-pyproject.toml") From d15bd286adaf10b81f85fbfc094f0253b9805e4f Mon Sep 17 00:00:00 2001 From: Michael Waskom Date: Fri, 10 Jan 2025 22:39:35 +0000 Subject: [PATCH 2/2] Remove debugging print and add an assertion about the context mount --- test/conftest.py | 2 +- test/image_test.py | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/test/conftest.py b/test/conftest.py index c9d46c595..392334bad 100644 --- a/test/conftest.py +++ b/test/conftest.py @@ -237,7 +237,7 @@ def __init__(self, blob_host, blobs, credentials): ( python_standalone_mount_name(version), api_pb2.DEPLOYMENT_NAMESPACE_GLOBAL, - ): f"mo-{version.replace('.', '')}" + ): f"mo-py{version.replace('.', '')}" for version in PYTHON_STANDALONE_VERSIONS }, } diff --git a/test/image_test.py b/test/image_test.py index 9c299a231..060380cfd 100644 --- a/test/image_test.py +++ b/test/image_test.py @@ -330,7 +330,7 @@ def test_from_registry_add_python(builder_version, servicer, client): with app.run(client=client): layers = get_image_layers(app.image.object_id, servicer) commands = layers[0].dockerfile_commands - print(commands) + assert layers[0].context_mount_id == "mo-py39" assert any("COPY /python/. /usr/local" in cmd for cmd in commands) assert any("ln -s /usr/local/bin/python3" in cmd for cmd in commands) @@ -341,7 +341,7 @@ def test_from_registry_add_python(builder_version, servicer, client): with app.run(client=client): layers = get_image_layers(app.image.object_id, servicer) commands = layers[0].dockerfile_commands - print(commands) + assert layers[0].context_mount_id == "mo-py313" assert any("COPY /python/. /usr/local" in cmd for cmd in commands) assert not any("ln -s /usr/local/bin/python3" in cmd for cmd in commands)