Skip to content

fix: fix PyTorchModel deployment crash on Windows #1212

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jan 7, 2020

Conversation

Serhiy-Shekhovtsov
Copy link
Contributor

Starting from version 1.2 deployment of PyTorchModel always initiates repacking when uploading the code. It is done by calling _upload_code with repack=True parameter.

On Windows, this will lead to a crash, because os.path.join method used to generate repacked_model_uri will use backslashes instead of slashes. So utils.repack_model will fail to parse it.

Description of changes:
Replaced os.path.join(bucket, key_prefix, "model.tar.gz") with '/'.join(...) for creating repacked_model_uri.

Merge Checklist

Put an x in the boxes that apply. You can also fill these out after creating the PR. If you're unsure about any of them, don't hesitate to ask. We're here to help! This is simply a reminder of what we are going to look for before merging your pull request.

  • I have read the CONTRIBUTING doc
  • I used the commit message format described in CONTRIBUTING
  • I have used the regional endpoint when creating S3 and/or STS clients (if appropriate)
  • I have added tests that prove my fix is effective or that my feature works (if appropriate)
  • I have checked that my tests are not configured for a specific region or account (if appropriate)
  • I have updated any necessary documentation, including READMEs and API docs (if appropriate)

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

Starting from version `1.2` deployment of PyTorchModel always initiates repacking when uploading the code.  It is done by calling `_upload_code` with `repack=True` parameter. On Windows, this will lead to a crash, because `os.path.join` method used to generate `repacked_model_uri` will use backslashes instead of slashes. So `utils.repack_model` will fail to parse it.
@sagemaker-bot
Copy link
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

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

@sagemaker-bot
Copy link
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

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

@sagemaker-bot
Copy link
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

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

@sagemaker-bot
Copy link
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

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

Copy link
Contributor

@laurenyu laurenyu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks for the contribution!

@laurenyu laurenyu merged commit 2e81f95 into aws:master Jan 7, 2020
knikure added a commit to knikure/sagemaker-python-sdk that referenced this pull request Oct 26, 2023
…- Galactus (aws#1212)

Co-authored-by: Gary Wang <[email protected]>
Co-authored-by: Gary Wang <[email protected]>
Co-authored-by: Raymond Liu <[email protected]>
Co-authored-by: John Barboza <[email protected]>
Co-authored-by: Malav Shastri <[email protected]>
Co-authored-by: Mufaddal Rohawala <[email protected]>
Co-authored-by: Mike Schneider <[email protected]>
Co-authored-by: Bhupendra Singh <[email protected]>
Co-authored-by: ci <ci>
Co-authored-by: Malav Shastri <[email protected]>
Co-authored-by: Keshav Chandak <[email protected]>
Co-authored-by: Zuoyuan Huang <[email protected]>
Co-authored-by: evakravi <[email protected]>
Co-authored-by: Keshav Chandak <[email protected]>
Co-authored-by: Alexander Pivovarov <[email protected]>
Co-authored-by: SSRraymond <[email protected]>
Co-authored-by: Ruilian Gao <[email protected]>
Co-authored-by: Ao Guo <[email protected]>
Co-authored-by: qidewenwhen <[email protected]>
Co-authored-by: mariumof <[email protected]>
Co-authored-by: matherit <[email protected]>
Co-authored-by: amzn-choeric <[email protected]>
Co-authored-by: Ao Guo <[email protected]>
Co-authored-by: Sally Seok <[email protected]>
Co-authored-by: Erick Benitez-Ramos <[email protected]>
Co-authored-by: Qingzi-Lan <[email protected]>
Co-authored-by: Sally Seok <[email protected]>
Co-authored-by: Manu Seth <[email protected]>
Co-authored-by: Miyoung <[email protected]>
Co-authored-by: Sarah Castillo <[email protected]>
Co-authored-by: EC2 Default User <[email protected]>
Co-authored-by: EC2 Default User <[email protected]>
Co-authored-by: EC2 Default User <[email protected]>
Co-authored-by: Xin Wang <[email protected]>
Co-authored-by: stacicho <[email protected]>
Co-authored-by: martinRenou <[email protected]>
Co-authored-by: jiapinw <[email protected]>
Co-authored-by: Akash Goel <[email protected]>
Co-authored-by: Joseph Zhang <[email protected]>
Co-authored-by: Harsha Reddy <[email protected]>
Co-authored-by: Haixin Wang <[email protected]>
Co-authored-by: Kalyani Nikure <[email protected]>
Co-authored-by: Xin Wang <[email protected]>
Co-authored-by: Gili Nachum <[email protected]>
Co-authored-by: Jose Pena <[email protected]>
Co-authored-by: cansun <[email protected]>
Co-authored-by: AWS-pratab <[email protected]>
Co-authored-by: shenlongtang <[email protected]>
Co-authored-by: Zach Kimberg <[email protected]>
Co-authored-by: chrivtho-github <[email protected]>
Co-authored-by: Justin <[email protected]>
Co-authored-by: Duc Trung Le <[email protected]>
Co-authored-by: HappyAmazonian <[email protected]>
Co-authored-by: cj-zhang <[email protected]>
Co-authored-by: Matthew <[email protected]>
Co-authored-by: Zach Kimberg <[email protected]>
Co-authored-by: Rohith Nadimpally <[email protected]>
Co-authored-by: rohithn1 <[email protected]>
Co-authored-by: Victor Zhu <[email protected]>
Co-authored-by: jbarz1 <[email protected]>
Co-authored-by: Mohan Gandhi <[email protected]>
Co-authored-by: Mohan Gandhi <[email protected]>
Co-authored-by: Barboza <[email protected]>
Co-authored-by: ruiliann666 <[email protected]>
fixes (aws#963)
fix: skip tensorflow local mode notebook test (aws#4060)
Fix TorchTensorSer/Deser (aws#969)
fix (aws#971)
fix local container mode (aws#972)
Fix auto detect (aws#979)
Fix routing fn (aws#981)
fix: tags for jumpstart model package models (aws#4061)
fix: pipeline variable kms key (aws#4065)
fix: jumpstart cache using sagemaker session s3 client (aws#4051)
fix: gated models unsupported region (aws#4069)
fix local container serialization (aws#989)
fix custom serialiazation with local container. Also remove a  lot of unused code (aws#994)
Fix custom serialization for local container mode (aws#1000)
fix pytorch version (aws#1001)
Fix unit test (aws#990)
Fix unit tests (aws#1018)
Fix happy hf test (aws#1026)
fix logic setup (aws#1034)
fixes (aws#1045)
Fix flake error in init (aws#1050)
fix (aws#1053)
fix: pipeline upsert failed to pass parallelism_config to update (aws#4066)
fix: temporarily skip kmeans notebook (aws#4092)
fixes (aws#1051)
Fix missing absolute import error (aws#1057)
Fix flake8 error in unit test (aws#1058)
fixes (aws#1056)
Fix flake8 error in integ test (aws#1060)
Fix black format error in test_pickle_dependencies (aws#1062)
Fix docstyle error under serve (aws#1065)
Fix docstyle error in builder failure (aws#1066)
fix black and flake8 formatting (aws#1069)
Fix format error (aws#1070)
Fix integ test (aws#1074)
fix: HuggingFaceProcessor parameterized instance_type when image_uri is absent (aws#4072)
fix: log message when sdk defaults not applied (aws#4104)
fix: handle bad jumpstart default session (aws#4109)
Fix the version information, whl and flake8 (aws#1085)
Fix JSON serializer error (aws#1088)
Fix unit test (aws#1091)
fix format (aws#1103)
Fix local mode predictor (aws#1107)
Fix DJLPredictor (aws#1108)
Fix modelbuilder unit tests (aws#1118)
fixes (aws#1136)
fixes (aws#1165)
fixes (aws#1166)
fix: auto ml integ tests and add flaky test markers (aws#4136)
fix model data for JumpStartModel (aws#4135)
fix: transform step  unit test (aws#4151)
fix: Update pipeline.py and selective_execution_config.py with small fixes (aws#1099)
fix: Fixed bug in _create_training_details (aws#4141)
fix: use correct line endings and s3 uris on windows (aws#4118)
fix: js tagging s3 prefix (aws#4167)
fix: Update Ec2 instance type to g5.4xlarge in test_huggingface_torch_distributed.py (aws#4181)
fix: import error in unsupported js regions (aws#4188)
fix: update local mode schema (aws#4185)
fix: fix flaky Inference Recommender integration tests (aws#4156)
fix: clone distribution in validate_distribution (aws#4205)
Fix hyperlinks in feature_processor.scheduler parameter descriptions (aws#4208)
Fix master merge formatting (aws#1186)
Fix master unit tests (aws#1203)
Fix djl unit tests (aws#1204)
Fix merge conflicts (aws#1217)
fix: fix URL links (aws#4217)
fix: bump urllib3 version (aws#4223)
fix: relax upper bound on urllib in local mode requirements (aws#4219)
fixes (aws#1224)
fix formatting (aws#1233)
fix byoc unit tests (aws#1235)
fix byoc unit tests (aws#1236)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants