Skip to content

Commit

Permalink
Fix bug with add_python='3.9' (#2751)
Browse files Browse the repository at this point in the history
* Fix bug with add_python='3.9'

* Remove debugging print and add an assertion about the context mount
  • Loading branch information
mwaskom authored Jan 10, 2025
1 parent e45e8a6 commit 831c384
Show file tree
Hide file tree
Showing 3 changed files with 33 additions and 2 deletions.
3 changes: 2 additions & 1 deletion modal/image.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
9 changes: 8 additions & 1 deletion test/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -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


Expand Down Expand Up @@ -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-py{version.replace('.', '')}"
for version in PYTHON_STANDALONE_VERSIONS
},
}

self.deployed_nfss = {}
Expand Down
23 changes: 23 additions & 0 deletions test/image_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
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)

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
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)


def test_image_pip_install_pyproject(builder_version, servicer, client):
pyproject_toml = os.path.join(os.path.dirname(__file__), "supports/test-pyproject.toml")

Expand Down

0 comments on commit 831c384

Please sign in to comment.