fix: construct correct code path and correctly tar up contents #84

dk19y · 2020-09-02T00:07:17Z

Fix PyTorch SDK Failure for DLC Containers

Incorrect use of paths results in SDK tests failing link

This PR

Fixes the issue
Updates framework version in buildspec
Updates test Docker to use DLC images directly.

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

sagemaker-bot · 2020-09-02T00:08:57Z

AWS CodeBuild CI Report

CodeBuild project: sagemaker-pytorch-inference-container-pr
Commit ID: 8e6d29d
Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

sagemaker-bot · 2020-09-02T00:12:37Z

AWS CodeBuild CI Report

CodeBuild project: sagemaker-pytorch-inference-toolkit-pr
Commit ID: 8e6d29d
Result: FAILED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

sagemaker-bot · 2020-09-02T00:31:21Z

AWS CodeBuild CI Report

CodeBuild project: sagemaker-pytorch-inference-container-pr
Commit ID: bb0acc5
Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

sagemaker-bot · 2020-09-02T00:31:48Z

AWS CodeBuild CI Report

CodeBuild project: sagemaker-pytorch-inference-toolkit-pr
Commit ID: bb0acc5
Result: FAILED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

sagemaker-bot · 2020-09-02T00:42:49Z

AWS CodeBuild CI Report

CodeBuild project: sagemaker-pytorch-inference-container-pr
Commit ID: c050581
Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

sagemaker-bot · 2020-09-02T00:43:14Z

AWS CodeBuild CI Report

CodeBuild project: sagemaker-pytorch-inference-toolkit-pr
Commit ID: c050581
Result: FAILED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

sagemaker-bot · 2020-09-02T00:56:18Z

AWS CodeBuild CI Report

CodeBuild project: sagemaker-pytorch-inference-container-pr
Commit ID: 5378d68
Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

sagemaker-bot · 2020-09-02T01:15:43Z

AWS CodeBuild CI Report

CodeBuild project: sagemaker-pytorch-inference-toolkit-pr
Commit ID: 5378d68
Result: FAILED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

sagemaker-bot · 2020-09-02T01:22:51Z

AWS CodeBuild CI Report

CodeBuild project: sagemaker-pytorch-inference-container-pr
Commit ID: 358b4cc
Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

sagemaker-bot · 2020-09-02T01:26:20Z

AWS CodeBuild CI Report

CodeBuild project: sagemaker-pytorch-inference-toolkit-pr
Commit ID: 358b4cc
Result: FAILED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

sagemaker-bot · 2020-09-02T01:32:04Z

AWS CodeBuild CI Report

CodeBuild project: sagemaker-pytorch-inference-container-pr
Commit ID: 77faf02
Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

sagemaker-bot · 2020-09-02T01:35:45Z

AWS CodeBuild CI Report

CodeBuild project: sagemaker-pytorch-inference-toolkit-pr
Commit ID: 77faf02
Result: FAILED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

sagemaker-bot · 2020-09-02T01:42:34Z

AWS CodeBuild CI Report

CodeBuild project: sagemaker-pytorch-inference-container-pr
Commit ID: 3e03702
Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

sagemaker-bot · 2020-09-02T02:38:45Z

AWS CodeBuild CI Report

CodeBuild project: sagemaker-pytorch-inference-toolkit-pr
Commit ID: 3e03702
Result: FAILED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

sagemaker-bot · 2020-09-02T05:16:29Z

AWS CodeBuild CI Report

CodeBuild project: sagemaker-pytorch-inference-container-pr
Commit ID: 67f9a23
Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

sagemaker-bot · 2020-09-02T05:20:05Z

AWS CodeBuild CI Report

CodeBuild project: sagemaker-pytorch-inference-toolkit-pr
Commit ID: 67f9a23
Result: FAILED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

sagemaker-bot · 2020-09-02T05:27:14Z

AWS CodeBuild CI Report

CodeBuild project: sagemaker-pytorch-inference-container-pr
Commit ID: 7cd8cbb
Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

sagemaker-bot · 2020-09-02T06:20:51Z

AWS CodeBuild CI Report

CodeBuild project: sagemaker-pytorch-inference-toolkit-pr
Commit ID: 7cd8cbb
Result: FAILED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

sagemaker-bot · 2020-09-02T07:23:50Z

AWS CodeBuild CI Report

CodeBuild project: sagemaker-pytorch-inference-container-pr
Commit ID: 508a8a2
Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

sagemaker-bot · 2020-09-02T08:44:01Z

AWS CodeBuild CI Report

CodeBuild project: sagemaker-pytorch-inference-toolkit-pr
Commit ID: 508a8a2
Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

metrizable

thanks for the submission. i included a few suggestions.

metrizable · 2020-09-02T14:57:19Z

src/sagemaker_pytorch_serving_container/torchserve.py

@@ -121,7 +122,7 @@ def _adapt_to_ts_format(handler_service):
        "--export-path",
        DEFAULT_TS_MODEL_DIRECTORY,
        "--extra-files",
-        os.path.join(environment.model_dir, environment.Environment().module_name + ".py"),
+        os.path.join(environment.model_dir, DEFAULT_TS_CODE_DIR, environment.Environment().module_name + ".py"),


use string interpolation here:

Suggested change

os.path.join(environment.model_dir, DEFAULT_TS_CODE_DIR, environment.Environment().module_name + ".py"),

os.path.join(environment.model_dir, DEFAULT_TS_CODE_DIR, "{}.py".format(environment.Environment().module_name)),

metrizable · 2020-09-02T14:58:03Z

test/integration/__init__.py

@@ -23,36 +23,41 @@
 cpu_sub_dir = 'model_cpu'
 gpu_sub_dir = 'model_gpu'
 eia_sub_dir = 'model_eia'
+code_sub_dir = 'code'


apply black formatter and favor double-quotes here and throughout

metrizable · 2020-09-02T14:58:25Z

test/integration/__init__.py


 model_cpu_dir = os.path.join(mnist_path, cpu_sub_dir)
-mnist_cpu_script = os.path.join(model_cpu_dir, 'mnist.py')
+mnist_cpu_script = os.path.join(model_cpu_dir, code_sub_dir, 'mnist.py')


standardize the quotes used.

Suggested change

mnist_cpu_script = os.path.join(model_cpu_dir, code_sub_dir, 'mnist.py')

mnist_cpu_script = os.path.join(model_cpu_dir, code_sub_dir, "mnist.py")

applying black formatter will standardize code and increase readability. we'll introduce black to this package to auto-format, increase code quality, and eliminate these nits

metrizable · 2020-09-02T15:00:06Z

test/unit/test_model_server.py

-        os.path.join(environment.model_dir, environment.Environment().module_name + ".py"),
+        os.path.join(environment.model_dir,
+                     torchserve.DEFAULT_TS_CODE_DIR,
+                     environment.Environment().module_name + ".py"),


Suggested change

environment.Environment().module_name + ".py"),

"{}.py".format(environment.Environment().module_name)),

metrizable · 2020-09-02T15:02:53Z

test/utils/file_utils.py

@@ -16,9 +16,12 @@
 import tarfile


-def make_tarfile(script, model, output_path, filename="model.tar.gz"):
+def make_tarfile(script, model, output_path, filename="model.tar.gz", script_path=None):


default to empty string and then later you can simplify code (plus it matches the type expected):

Suggested change

def make_tarfile(script, model, output_path, filename="model.tar.gz", script_path=None):

def make_tarfile(script, model, output_path, filename="model.tar.gz", script_path=""):

metrizable · 2020-09-02T15:19:51Z

test/utils/file_utils.py

+        if(script_path):
+            tar.add(script, arcname=os.path.join(script_path, os.path.basename(script)))
+        else:
+            tar.add(script, arcname=os.path.basename(script))


assuming script_path is defaulted to empty string:

Suggested change

if(script_path):

tar.add(script, arcname=os.path.join(script_path, os.path.basename(script)))

else:

tar.add(script, arcname=os.path.basename(script))

tar.add(script, arcname=os.path.join(script_path, os.path.basename(script)))

this is because os.path.join with empty string arg has no effect on the resulting path:

>>> os.path.join("", "/tmp/foo") '/tmp/foo' >>> os.path.join("", "/tmp", "foo") '/tmp/foo' >>> os.path.join("", "tmp", "foo") 'tmp/foo'

metrizable

fast-follow on suggestions

sagemaker-bot · 2020-12-10T18:25:59Z

AWS CodeBuild CI Report

CodeBuild project: sagemaker-pytorch-inference-toolkit-pr
Commit ID: 508a8a2
Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

Fix PyTorch SDK Failure

8e6d29d

Fix Unit tests

bb0acc5

Flake fix

c050581

Flake

5378d68

Move code files to dir

358b4cc

Integ test fix

77faf02

Fix EIA code dir

3e03702

Fix paths

67f9a23

Fix preserve_script_path arg

7cd8cbb

Update script path

508a8a2

metrizable reviewed Sep 2, 2020

View reviewed changes

metrizable approved these changes Sep 2, 2020

View reviewed changes

metrizable merged commit 9a6869e into aws:master Sep 2, 2020

metrizable changed the title ~~Fix PyTorch SDK Failure for DLC Containers~~ fix: construct correct code path and correctly tar up contents Sep 2, 2020

dk19y mentioned this pull request Sep 3, 2020

[PyTorch] [Inference] Integ tests fix for code dir path for PyTorch SM SDK aws/deep-learning-containers#563

Merged

	os.path.join(environment.model_dir, DEFAULT_TS_CODE_DIR, environment.Environment().module_name + ".py"),
	os.path.join(environment.model_dir, DEFAULT_TS_CODE_DIR, "{}.py".format(environment.Environment().module_name)),

	mnist_cpu_script = os.path.join(model_cpu_dir, code_sub_dir, 'mnist.py')
	mnist_cpu_script = os.path.join(model_cpu_dir, code_sub_dir, "mnist.py")

	environment.Environment().module_name + ".py"),
	"{}.py".format(environment.Environment().module_name)),

	def make_tarfile(script, model, output_path, filename="model.tar.gz", script_path=None):
	def make_tarfile(script, model, output_path, filename="model.tar.gz", script_path=""):

fix: construct correct code path and correctly tar up contents #84

fix: construct correct code path and correctly tar up contents #84

Uh oh!

Conversation

dk19y commented Sep 2, 2020

Uh oh!

sagemaker-bot commented Sep 2, 2020

AWS CodeBuild CI Report

Uh oh!

sagemaker-bot commented Sep 2, 2020

AWS CodeBuild CI Report

Uh oh!

sagemaker-bot commented Sep 2, 2020

AWS CodeBuild CI Report

Uh oh!

sagemaker-bot commented Sep 2, 2020

AWS CodeBuild CI Report

Uh oh!

sagemaker-bot commented Sep 2, 2020

AWS CodeBuild CI Report

Uh oh!

sagemaker-bot commented Sep 2, 2020

AWS CodeBuild CI Report

Uh oh!

sagemaker-bot commented Sep 2, 2020

AWS CodeBuild CI Report

Uh oh!

sagemaker-bot commented Sep 2, 2020

AWS CodeBuild CI Report

Uh oh!

sagemaker-bot commented Sep 2, 2020

AWS CodeBuild CI Report

Uh oh!

sagemaker-bot commented Sep 2, 2020

AWS CodeBuild CI Report

Uh oh!

sagemaker-bot commented Sep 2, 2020

AWS CodeBuild CI Report

Uh oh!

sagemaker-bot commented Sep 2, 2020

AWS CodeBuild CI Report

Uh oh!

sagemaker-bot commented Sep 2, 2020

AWS CodeBuild CI Report

Uh oh!

sagemaker-bot commented Sep 2, 2020

AWS CodeBuild CI Report

Uh oh!

sagemaker-bot commented Sep 2, 2020

AWS CodeBuild CI Report

Uh oh!

sagemaker-bot commented Sep 2, 2020

AWS CodeBuild CI Report

Uh oh!

sagemaker-bot commented Sep 2, 2020

AWS CodeBuild CI Report

Uh oh!

sagemaker-bot commented Sep 2, 2020

AWS CodeBuild CI Report

Uh oh!

sagemaker-bot commented Sep 2, 2020

AWS CodeBuild CI Report

Uh oh!

sagemaker-bot commented Sep 2, 2020

AWS CodeBuild CI Report

Uh oh!

metrizable left a comment

Choose a reason for hiding this comment

Uh oh!

metrizable Sep 2, 2020

Choose a reason for hiding this comment

Uh oh!

metrizable Sep 2, 2020

Choose a reason for hiding this comment

Uh oh!

metrizable Sep 2, 2020

Choose a reason for hiding this comment

Uh oh!

metrizable Sep 2, 2020

Choose a reason for hiding this comment