Skip to content
This repository was archived by the owner on Aug 26, 2020. It is now read-only.

Commit 1d2c04e

Browse files
authored
fix: extract module to correct location in download_and_install (#261)
1 parent a4519f8 commit 1d2c04e

File tree

2 files changed

+27
-15
lines changed

2 files changed

+27
-15
lines changed

src/sagemaker_containers/_modules.py

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -155,10 +155,8 @@ def download_and_install(uri, name=DEFAULT_MODULE_NAME, cache=True):
155155

156156
if not should_use_cache:
157157
with _files.tmpdir() as tmpdir:
158-
dst = os.path.join(tmpdir, "tar_file")
159-
_files.download_and_extract(uri, dst)
160158
module_path = os.path.join(tmpdir, "module_dir")
161-
os.makedirs(module_path)
159+
_files.download_and_extract(uri, module_path)
162160
prepare(module_path, name)
163161
install(module_path)
164162

test/unit/test_modules.py

Lines changed: 26 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -218,17 +218,31 @@ def test_import_module(reload, import_module, install, download_and_extract):
218218
reload.assert_called_with(import_module(_modules.DEFAULT_MODULE_NAME))
219219

220220

221-
def test_download_and_install_local_directory():
221+
@patch("sagemaker_containers._modules.exists", return_value=False)
222+
@patch("sagemaker_containers._files.tmpdir")
223+
@patch("sagemaker_containers._files.download_and_extract")
224+
@patch("sagemaker_containers._modules.prepare")
225+
@patch("sagemaker_containers._modules.install")
226+
def test_download_and_install(install, prepare, download_and_extract, files_tmpdir, module_exists):
227+
files_tmpdir.return_value.__enter__.return_value = "tmp"
228+
uri = "s3://foo/bar"
229+
_modules.download_and_install(uri)
230+
231+
module_path = os.path.join("tmp", "module_dir")
232+
download_and_extract.assert_called_with(uri, module_path)
233+
prepare.assert_called_with(module_path, "default_user_module_name")
234+
install.assert_called_with(module_path)
235+
236+
237+
@patch("sagemaker_containers._files.s3_download")
238+
@patch("tarfile.open")
239+
@patch("sagemaker_containers._modules.prepare")
240+
@patch("sagemaker_containers._modules.install")
241+
def test_download_and_install_local_directory(install, prepare, tarfile, s3_download):
222242
uri = "/opt/ml/input/data/code/sourcedir.tar.gz"
243+
_modules.download_and_install(uri)
223244

224-
with patch("sagemaker_containers._files.s3_download") as s3_download, patch(
225-
"tarfile.open"
226-
) as tarfile, patch("sagemaker_containers._modules.prepare") as prepare, patch(
227-
"sagemaker_containers._modules.install"
228-
) as install:
229-
_modules.download_and_install(uri)
230-
231-
s3_download.assert_not_called()
232-
tarfile.assert_called_with(name="/opt/ml/input/data/code/sourcedir.tar.gz", mode="r:gz")
233-
prepare.assert_called_once()
234-
install.assert_called_once()
245+
s3_download.assert_not_called()
246+
tarfile.assert_called_with(name="/opt/ml/input/data/code/sourcedir.tar.gz", mode="r:gz")
247+
prepare.assert_called_once()
248+
install.assert_called_once()

0 commit comments

Comments
 (0)