From d3a60dc1e56981da4d5f5447819e08b02a2807d3 Mon Sep 17 00:00:00 2001 From: ivanmkc Date: Tue, 19 Apr 2022 07:42:05 -0400 Subject: [PATCH 1/3] fix: Removed dirs_exist_ok parameter as it's not backwards compatible --- google/cloud/aiplatform/utils/source_utils.py | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/google/cloud/aiplatform/utils/source_utils.py b/google/cloud/aiplatform/utils/source_utils.py index 2a13daf452..0d82636115 100644 --- a/google/cloud/aiplatform/utils/source_utils.py +++ b/google/cloud/aiplatform/utils/source_utils.py @@ -171,7 +171,14 @@ def make_package(self, package_directory: str) -> str: fp.write(setup_py_output) if os.path.isdir(self.script_path): - shutil.copytree(self.script_path, trainer_path, dirs_exist_ok=True) + # Remove destination path if it already exists + shutil.rmtree(trainer_path) + + # Create destination path + os.makedirs(trainer_path) + + # Copy folder recursively + shutil.copytree(src=self.script_path, dst=trainer_path) else: # The module that will contain the script script_out_path = trainer_path / f"{self.task_module_name}.py" From f12c43ad406e3ca416efaa710c7cbe70833dfaa6 Mon Sep 17 00:00:00 2001 From: ivanmkc Date: Mon, 23 May 2022 18:16:11 -0400 Subject: [PATCH 2/3] Added unit tests and fixed bug --- google/cloud/aiplatform/utils/source_utils.py | 3 - tests/unit/aiplatform/test_training_utils.py | 78 +++++++++++++++++++ 2 files changed, 78 insertions(+), 3 deletions(-) diff --git a/google/cloud/aiplatform/utils/source_utils.py b/google/cloud/aiplatform/utils/source_utils.py index 0d82636115..dc3c14a759 100644 --- a/google/cloud/aiplatform/utils/source_utils.py +++ b/google/cloud/aiplatform/utils/source_utils.py @@ -174,9 +174,6 @@ def make_package(self, package_directory: str) -> str: # Remove destination path if it already exists shutil.rmtree(trainer_path) - # Create destination path - os.makedirs(trainer_path) - # Copy folder recursively shutil.copytree(src=self.script_path, dst=trainer_path) else: diff --git a/tests/unit/aiplatform/test_training_utils.py b/tests/unit/aiplatform/test_training_utils.py index 99d49b7ead..9cb7933981 100644 --- a/tests/unit/aiplatform/test_training_utils.py +++ b/tests/unit/aiplatform/test_training_utils.py @@ -16,11 +16,15 @@ # from importlib import reload +import filecmp +import glob import json import os import pytest +import tempfile from google.cloud.aiplatform.training_utils import environment_variables +from google.cloud.aiplatform.utils import source_utils from unittest import mock _TEST_TRAINING_DATA_URI = "gs://training-data-uri" @@ -203,3 +207,77 @@ def test_http_handler_port(self): def test_http_handler_port_none(self): reload(environment_variables) assert environment_variables.http_handler_port is None + + @pytest.fixture() + def mock_temp_file_name(self): + # Create random files + # tmpdirname = tempfile.TemporaryDirectory() + file = tempfile.NamedTemporaryFile() + + with open(file.name, "w") as handle: + handle.write("test") + + yield file.name + + file.close() + + def test_package_file(self, mock_temp_file_name): + # Test that the packager properly copies the source file to the destination file + + packager = source_utils._TrainingScriptPythonPackager( + script_path=mock_temp_file_name + ) + + with tempfile.TemporaryDirectory() as destination_directory_name: + _ = packager.make_package(package_directory=destination_directory_name) + + # Check that contents of source_distribution_path is the same as destination_directory_name + destination_inner_path = f"{destination_directory_name}/{packager._TRAINER_FOLDER}/{packager._ROOT_MODULE}/{packager.task_module_name}.py" + + assert filecmp.cmp( + mock_temp_file_name, destination_inner_path, shallow=False + ) + + @pytest.fixture() + def mock_temp_folder_name(self): + # Create random folder + folder = tempfile.TemporaryDirectory() + + file = tempfile.NamedTemporaryFile(dir=folder.name) + + # Create random file in the folder + with open(file.name, "w") as handle: + handle.write("test") + + yield folder.name + + file.close() + + folder.cleanup() + + def test_package_folder(self, mock_temp_folder_name): + # Test that the packager properly copies the source folder to the destination folder + + packager = source_utils._TrainingScriptPythonPackager( + script_path=mock_temp_folder_name + ) + + with tempfile.TemporaryDirectory() as destination_directory_name: + # Add an existing file into the destination directory to check if it gets deleted + existing_file = tempfile.NamedTemporaryFile(dir=destination_directory_name) + + with open(existing_file.name, "w") as handle: + handle.write("existing") + + _ = packager.make_package(package_directory=destination_directory_name) + + # Check that contents of source_distribution_path is the same as destination_directory_name + destination_inner_path = f"{destination_directory_name}/{packager._TRAINER_FOLDER}/{packager._ROOT_MODULE}" + + dcmp = filecmp.dircmp(mock_temp_folder_name, destination_inner_path) + + assert len(dcmp.diff_files) == 0 + assert len(dcmp.left_only) == 0 + assert len(dcmp.right_only) == 0 + + existing_file.close() From 73b287beaf8d75b8875c34bb1208afa991c4bd4b Mon Sep 17 00:00:00 2001 From: ivanmkc Date: Thu, 9 Jun 2022 16:05:24 -0400 Subject: [PATCH 3/3] Removed unneeded import --- tests/unit/aiplatform/test_training_utils.py | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/unit/aiplatform/test_training_utils.py b/tests/unit/aiplatform/test_training_utils.py index 9cb7933981..24a60d95ff 100644 --- a/tests/unit/aiplatform/test_training_utils.py +++ b/tests/unit/aiplatform/test_training_utils.py @@ -17,7 +17,6 @@ from importlib import reload import filecmp -import glob import json import os import pytest