Skip to content

Commit

Permalink
fix: copy dependencies into new folder when repacking model (#1021)
Browse files Browse the repository at this point in the history
  • Loading branch information
laurenyu committed Sep 10, 2019
1 parent de676a1 commit e62dd2e
Show file tree
Hide file tree
Showing 2 changed files with 12 additions and 7 deletions.
6 changes: 4 additions & 2 deletions src/sagemaker/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -468,10 +468,12 @@ def _create_or_update_code_dir(
shutil.copy2(inference_script, code_dir)

for dependency in dependencies:
lib_dir = os.path.join(code_dir, "lib")

This comment has been minimized.

Copy link
@anotinelg

anotinelg Sep 20, 2019

why do you put the code in a lib directory?

when my endpoint is launched, there are import failures as code into the /opt/ml/code/lib is not found from the main script copied into /opt/ml/code. Is there something I miss? or is this a bug?

This comment has been minimized.

Copy link
@laurenyu

laurenyu Sep 20, 2019

Author Contributor

@anotinelg I was following the structure described in https://github.com/aws/sagemaker-tensorflow-serving-container/#prepost-processing. However, if it is presenting issues for you, please open an issue against this repository

This comment has been minimized.

Copy link
@anotinelg

anotinelg Sep 23, 2019

It seems that sagemaker-mxnet-serving-container does not follow the same convention ( https://github.com/aws/sagemaker-tensorflow-serving-container/#prepost-processing). Is it a problem of the sagemaker-python-sdk API or the the sagemaker-mxnet-serving-container one? Where do you think the possible bug must be added?

if os.path.isdir(dependency):
shutil.copytree(dependency, code_dir)
shutil.copytree(dependency, os.path.join(lib_dir, os.path.basename(dependency)))
else:
shutil.copy2(dependency, code_dir)
os.mkdir(lib_dir)
shutil.copy2(dependency, lib_dir)


def _extract_model(model_uri, sagemaker_session, tmp):
Expand Down
13 changes: 8 additions & 5 deletions tests/unit/test_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -344,7 +344,7 @@ def test_repack_model_without_source_dir(tmp, fake_s3):
[
"model-dir/model",
"dependencies/a",
"dependencies/b",
"dependencies/some/dir/b",
"source-dir/inference.py",
"source-dir/this-file-should-not-be-included.py",
],
Expand All @@ -355,16 +355,19 @@ def test_repack_model_without_source_dir(tmp, fake_s3):
sagemaker.utils.repack_model(
inference_script=os.path.join(tmp, "source-dir/inference.py"),
source_directory=None,
dependencies=[os.path.join(tmp, "dependencies/a"), os.path.join(tmp, "dependencies/b")],
dependencies=[
os.path.join(tmp, "dependencies/a"),
os.path.join(tmp, "dependencies/some/dir"),
],
model_uri="s3://fake/location",
repacked_model_uri="s3://destination-bucket/model.tar.gz",
sagemaker_session=fake_s3.sagemaker_session,
)

assert list_tar_files(fake_s3.fake_upload_path, tmp) == {
"/model",
"/code/a",
"/code/b",
"/code/lib/a",
"/code/lib/dir/b",
"/code/inference.py",
}

Expand Down Expand Up @@ -449,7 +452,7 @@ def test_repack_model_from_file_to_file(tmp):
sagemaker_session,
)

assert list_tar_files(destination_path, tmp) == {"/code/a", "/code/inference.py", "/model"}
assert list_tar_files(destination_path, tmp) == {"/code/lib/a", "/code/inference.py", "/model"}


def test_repack_model_with_inference_code_should_replace_the_code(tmp, fake_s3):
Expand Down

0 comments on commit e62dd2e

Please sign in to comment.