Skip to content

Commit

Permalink
Clean up a little bit of 3.8-only code (#2597)
Browse files Browse the repository at this point in the history
* Clean up a little bit of 3.8-only code

* Revert a change that actually requires 3.9
  • Loading branch information
mwaskom authored Dec 3, 2024
1 parent a00b057 commit f417cc3
Show file tree
Hide file tree
Showing 7 changed files with 29 additions and 36 deletions.
2 changes: 1 addition & 1 deletion modal/cli/import_refs.py
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ def import_file_or_module(file_or_module: str):
# when using a script path, that scripts directory should also be on the path as it is
# with `python some/script.py`
full_path = Path(file_or_module).resolve()
if "." in full_path.name[:-3]: # use removesuffix once we drop 3.8 support
if "." in full_path.name.removesuffix(".py"):
raise InvalidError(
f"Invalid Modal source filename: {full_path.name!r}."
"\n\nSource filename cannot contain additional period characters."
Expand Down
5 changes: 2 additions & 3 deletions modal/functions.py
Original file line number Diff line number Diff line change
Expand Up @@ -912,9 +912,8 @@ async def _load(self: _Function, resolver: Resolver, existing_object_id: Optiona
function_data.ranked_functions.extend(ranked_functions)
function_definition = None # function_definition is not used in this case
else:
# TODO(irfansharif): Assert on this specific type once
# we get rid of python 3.8.
# assert isinstance(gpu, GPU_T) # includes the case where gpu==None case
# TODO(irfansharif): Assert on this specific type once we get rid of python 3.9.
# assert isinstance(gpu, GPU_T) # includes the case where gpu==None case
function_definition.resources.CopyFrom(
convert_fn_config_to_resources_config(
cpu=cpu, memory=memory, gpu=gpu, ephemeral_disk=ephemeral_disk
Expand Down
3 changes: 1 addition & 2 deletions modal/mount.py
Original file line number Diff line number Diff line change
Expand Up @@ -749,8 +749,7 @@ def get_auto_mounts() -> list[_Mount]:
continue

for local_path, remote_path in mount_paths:
# TODO: use is_relative_to once we deprecate Python 3.8
if any(str(local_path).startswith(str(p)) for p in SYS_PREFIXES) or _is_modal_path(remote_path):
if any(local_path.is_relative_to(p) for p in SYS_PREFIXES) or _is_modal_path(remote_path):
# skip any module that has paths in SYS_PREFIXES, or would overwrite the modal Package in the container
break
else:
Expand Down
7 changes: 1 addition & 6 deletions modal/volume.py
Original file line number Diff line number Diff line change
Expand Up @@ -677,16 +677,11 @@ def find_open_file_for_pid(pid: str) -> Optional[str]:
cmdline = " ".join([part.decode() for part in parts]).rstrip(" ")

cwd = PurePosixPath(os.readlink(f"/proc/{pid}/cwd"))
# NOTE(staffan): Python 3.8 doesn't have is_relative_to(), so we're stuck with catching ValueError until
# we drop Python 3.8 support.
try:
_rel_cwd = cwd.relative_to(mount_path)
if cwd.is_relative_to(mount_path):
if pid == self_pid:
return "cwd is inside volume"
else:
return f"cwd of '{cmdline}' is inside volume"
except ValueError:
pass

for fd in os.listdir(f"/proc/{pid}/fd"):
try:
Expand Down
2 changes: 1 addition & 1 deletion modal_global_objects/mounts/python_standalone.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ def publish_python_standalone_mount(client, version: str) -> None:
release, full_version = PYTHON_STANDALONE_VERSIONS[version]

libc = "gnu"
arch = "x86_64" if version == "3.8" else "x86_64_v3"
arch = "x86_64_v3"
url = (
"https://github.com/indygreg/python-build-standalone/releases/download"
+ f"/{release}/cpython-{full_version}+{release}-{arch}-unknown-linux-gnu-install_only.tar.gz"
Expand Down
6 changes: 2 additions & 4 deletions test/container_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -1116,10 +1116,8 @@ def test_cli(servicer, credentials):
stderr = ret.stderr.decode()
if ret.returncode != 0:
raise Exception(f"Failed with {ret.returncode} stdout: {stdout} stderr: {stderr}")

if sys.version_info[:2] != (3, 8): # Skip on Python 3.8 as we'll have PendingDeprecationError messages
assert stdout == ""
assert stderr == ""
assert stdout == ""
assert stderr == ""


@skip_github_non_linux
Expand Down
40 changes: 21 additions & 19 deletions test/image_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -1031,32 +1031,34 @@ def mock_base_image_config(group, version):
}
return config[group]

# TODO use a single with statement and tuple of managers when we drop Py3.8
test_requirements = str(test_dir / "supports" / "test-requirements.txt")
with mock.patch("modal.image._get_modal_requirements_path", lambda *_, **__: test_requirements):
with mock.patch("modal.image._dockerhub_python_version", lambda *_, **__: "3.11.0"):
with mock.patch("modal.image._base_image_config", mock_base_image_config):
with mock.patch("test.conftest.ImageBuilderVersion", Literal["2000.01"]):
with mock.patch("modal.image.ImageBuilderVersion", Literal["2000.01"]):
with Client(servicer.client_addr, api_pb2.CLIENT_TYPE_CLIENT, credentials) as client:
with modal_config():
with app.run(client=client):
assert servicer.image_builder_versions
for version in servicer.image_builder_versions.values():
assert version == "2000.01"
with (
mock.patch("modal.image._get_modal_requirements_path", lambda *_, **__: test_requirements),
mock.patch("modal.image._dockerhub_python_version", lambda *_, **__: "3.11.0"),
mock.patch("modal.image._base_image_config", mock_base_image_config),
mock.patch("test.conftest.ImageBuilderVersion", Literal["2000.01"]),
mock.patch("modal.image.ImageBuilderVersion", Literal["2000.01"]),
Client(servicer.client_addr, api_pb2.CLIENT_TYPE_CLIENT, credentials) as client,
modal_config(),
app.run(client=client),
):
assert servicer.image_builder_versions
for version in servicer.image_builder_versions.values():
assert version == "2000.01"


def test_image_builder_supported_versions(servicer, credentials):
app = App(image=Image.debian_slim())
app.function()(dummy)

# TODO use a single with statement and tuple of managers when we drop Py3.8
with pytest.raises(VersionError, match=r"This version of the modal client supports.+{'2000.01'}"):
with mock.patch("modal.image.ImageBuilderVersion", Literal["2000.01"]):
with mock.patch("test.conftest.ImageBuilderVersion", Literal["2023.11"]):
with Client(servicer.client_addr, api_pb2.CLIENT_TYPE_CLIENT, credentials) as client:
with app.run(client=client):
pass
with (
pytest.raises(VersionError, match=r"This version of the modal client supports.+{'2000.01'}"),
mock.patch("modal.image.ImageBuilderVersion", Literal["2000.01"]),
mock.patch("test.conftest.ImageBuilderVersion", Literal["2023.11"]),
Client(servicer.client_addr, api_pb2.CLIENT_TYPE_CLIENT, credentials) as client,
app.run(client=client),
):
pass


@pytest.fixture
Expand Down

0 comments on commit f417cc3

Please sign in to comment.