From f417cc3b15dcb4c36cf96db258418a1f97e97037 Mon Sep 17 00:00:00 2001 From: Michael Waskom Date: Tue, 3 Dec 2024 11:28:34 -0500 Subject: [PATCH] Clean up a little bit of 3.8-only code (#2597) * Clean up a little bit of 3.8-only code * Revert a change that actually requires 3.9 --- modal/cli/import_refs.py | 2 +- modal/functions.py | 5 +-- modal/mount.py | 3 +- modal/volume.py | 7 +--- .../mounts/python_standalone.py | 2 +- test/container_test.py | 6 +-- test/image_test.py | 40 ++++++++++--------- 7 files changed, 29 insertions(+), 36 deletions(-) diff --git a/modal/cli/import_refs.py b/modal/cli/import_refs.py index bfc0362f4..7479c4fbe 100644 --- a/modal/cli/import_refs.py +++ b/modal/cli/import_refs.py @@ -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." diff --git a/modal/functions.py b/modal/functions.py index 712202461..f04d4aef4 100644 --- a/modal/functions.py +++ b/modal/functions.py @@ -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 diff --git a/modal/mount.py b/modal/mount.py index 57aa8820e..f7e602bd8 100644 --- a/modal/mount.py +++ b/modal/mount.py @@ -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: diff --git a/modal/volume.py b/modal/volume.py index 6731e6b76..02cec5805 100644 --- a/modal/volume.py +++ b/modal/volume.py @@ -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: diff --git a/modal_global_objects/mounts/python_standalone.py b/modal_global_objects/mounts/python_standalone.py index 9c7690ebe..9d75550f3 100644 --- a/modal_global_objects/mounts/python_standalone.py +++ b/modal_global_objects/mounts/python_standalone.py @@ -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" diff --git a/test/container_test.py b/test/container_test.py index 3ca5edbcc..fefff9072 100644 --- a/test/container_test.py +++ b/test/container_test.py @@ -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 diff --git a/test/image_test.py b/test/image_test.py index d6e2b0372..f633549a2 100644 --- a/test/image_test.py +++ b/test/image_test.py @@ -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