fix: local mode deletion of temp files on job end #3017

jmahlik · 2022-03-22T03:53:21Z

Issue #, if available:
closes #2527

This is a clone of #2644 but has allow commits from maintainers on so should be easier to merge.

Description of changes:

Add try except around file cleanup in sagemaker.local.utils.copy_directory_structure. This was causing local mode to fail when docker (running as root) writes files to a mounted dir and the user does not have permission to remove the files.

The same pattern is followed here.

Testing done:

Add test for failed removal, behaves as expected.

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.

General

I have read the CONTRIBUTING doc
I certify that the changes I am introducing will be backword compatible, and I have discussed concerns about this, if any, with the Python SDK team
I used the commit message format described in CONTRIBUTING
I have passed the region in to all S3 and STS clients that I've initialized as part of this change.
I have updated any necessary documentation, including READMEs and API docs (if appropriate)

Tests

I have added tests that prove my fix is effective or that my feature works (if appropriate)
I have added unit and/or integration tests as appropriate to ensure backward compatibility of the changes
I have checked that my tests are not configured for a specific region or account (if appropriate)
I have used unique_name_from_base to create resource names in integ tests (if appropriate)

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

codecov-commenter · 2022-03-22T04:09:49Z

Codecov Report

Merging #3017 (aa5d3b2) into master (11ff7c0) will decrease coverage by 0.81%.
The diff coverage is 80.00%.

@@            Coverage Diff             @@
##           master    #3017      +/-   ##
==========================================
- Coverage   89.55%   88.75%   -0.81%     
==========================================
  Files         960      226     -734     
  Lines       88796    21967   -66829     
==========================================
- Hits        79523    19497   -60026     
+ Misses       9273     2470    -6803

Impacted Files	Coverage Δ
src/sagemaker/local/utils.py	`93.33% <80.00%> (ø)`
...thon3.8/site-packages/sagemaker/hyperparameters.py
...n3.7/site-packages/sagemaker/lineage/_api_types.py
...n3.9/site-packages/sagemaker/sklearn/processing.py
...agemaker/cli/compatibility/v2/modifiers/airflow.py
...hon3.9/site-packages/sagemaker/xgboost/__init__.py
...3.9/site-packages/sagemaker/wrangler/processing.py
.../lib/python3.10/site-packages/sagemaker/clarify.py
...s/sagemaker/workflow/pipeline_experiment_config.py
...te-packages/sagemaker/workflow/step_collections.py
... and 1177 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

sagemaker-bot · 2022-03-22T04:19:22Z

AWS CodeBuild CI Report

CodeBuild project: sagemaker-python-sdk-unit-tests
Commit ID: 2ea19d8
Result: SUCCEEDED
Build Logs (available for 30 days)

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

sagemaker-bot · 2022-03-22T05:13:36Z

AWS CodeBuild CI Report

CodeBuild project: sagemaker-python-sdk-local-mode-tests
Commit ID: 2ea19d8
Result: SUCCEEDED
Build Logs (available for 30 days)

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

sagemaker-bot · 2022-03-22T06:41:42Z

AWS CodeBuild CI Report

CodeBuild project: sagemaker-python-sdk-slow-tests
Commit ID: 2ea19d8
Result: FAILED
Build Logs (available for 30 days)

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

vikas0203

/bot run all

sagemaker-bot · 2022-10-26T23:11:44Z

AWS CodeBuild CI Report

CodeBuild project: sagemaker-python-sdk-unit-tests
Commit ID: 2ea19d8
Result: FAILED
Build Logs (available for 30 days)

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

sagemaker-bot · 2022-10-26T23:55:45Z

AWS CodeBuild CI Report

CodeBuild project: sagemaker-python-sdk-notebook-tests
Commit ID: 2ea19d8
Result: SUCCEEDED
Build Logs (available for 30 days)

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

navinsoni

@jmahlik Thanks for your contribution. Can you please update your PR and resolve conflicts.

jmahlik · 2022-12-06T14:06:18Z

@jmahlik Thanks for your contribution. Can you please update your PR and resolve conflicts.

My hard disk failed recently so I have to set up a new vm. It might take a bit. Open to you editing or cherry picking this in the meantime if you'd like? It should be open. Been trying to get this merged since September 2021.

jmahlik · 2022-12-14T04:20:18Z

@jmahlik Thanks for your contribution. Can you please update your PR and resolve conflicts.

@navinsoni rebased

navinsoni

/bot run all

navinsoni · 2022-12-15T21:11:08Z

@jmahlik I have approved the PR. I will merge it as soon as all tests pass.

sagemaker-bot · 2022-12-19T19:11:13Z

AWS CodeBuild CI Report

CodeBuild project: sagemaker-python-sdk-unit-tests
Commit ID: 6a3cea9
Result: SUCCEEDED
Build Logs (available for 30 days)

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

sagemaker-bot · 2023-01-05T23:34:58Z

AWS CodeBuild CI Report

CodeBuild project: sagemaker-python-sdk-unit-tests
Commit ID: aa5d3b2
Result: SUCCEEDED
Build Logs (available for 30 days)

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

sagemaker-bot · 2023-01-05T23:37:22Z

AWS CodeBuild CI Report

CodeBuild project: sagemaker-python-sdk-local-mode-tests
Commit ID: aa5d3b2
Result: SUCCEEDED
Build Logs (available for 30 days)

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

sagemaker-bot · 2023-01-05T23:49:18Z

AWS CodeBuild CI Report

CodeBuild project: sagemaker-python-sdk-pr
Commit ID: aa5d3b2
Result: SUCCEEDED
Build Logs (available for 30 days)

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

sagemaker-bot · 2023-01-05T23:56:04Z

AWS CodeBuild CI Report

CodeBuild project: sagemaker-python-sdk-notebook-tests
Commit ID: aa5d3b2
Result: FAILED
Build Logs (available for 30 days)

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

sagemaker-bot · 2023-01-06T01:48:24Z

AWS CodeBuild CI Report

CodeBuild project: sagemaker-python-sdk-slow-tests
Commit ID: aa5d3b2
Result: FAILED
Build Logs (available for 30 days)

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

jmahlik · 2023-01-11T18:30:11Z

@navinsoni looks like some unrelated tests are falling on this one. Let me know if you'd like me to rebase or anything.

claytonparnell

/bot run slow-tests

claytonparnell · 2023-01-18T16:58:37Z

@jmahlik - looks like just some intermittent failures that we're working on resolving, should pass on retry. I'll get those passing so we can merge this in. Thanks again for contributing!

sagemaker-bot · 2023-01-18T17:54:09Z

AWS CodeBuild CI Report

CodeBuild project: sagemaker-python-sdk-notebook-tests
Commit ID: aa5d3b2
Result: FAILED
Build Logs (available for 30 days)

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

sagemaker-bot · 2023-01-18T19:42:53Z

AWS CodeBuild CI Report

CodeBuild project: sagemaker-python-sdk-slow-tests
Commit ID: aa5d3b2
Result: FAILED
Build Logs (available for 30 days)

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

trajanikant

/bot run notebook-tests, slow-tests

sagemaker-bot · 2023-02-08T21:16:11Z

AWS CodeBuild CI Report

CodeBuild project: sagemaker-python-sdk-notebook-tests
Commit ID: aa5d3b2
Result: SUCCEEDED
Build Logs (available for 30 days)

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

sagemaker-bot · 2023-02-08T22:55:34Z

AWS CodeBuild CI Report

CodeBuild project: sagemaker-python-sdk-slow-tests
Commit ID: aa5d3b2
Result: FAILED
Build Logs (available for 30 days)

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

sagemaker-bot · 2023-02-10T22:28:59Z

AWS CodeBuild CI Report

CodeBuild project: sagemaker-python-sdk-slow-tests
Commit ID: aa5d3b2
Result: FAILED
Build Logs (available for 30 days)

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

sagemaker-bot · 2023-02-16T17:27:48Z

AWS CodeBuild CI Report

CodeBuild project: sagemaker-python-sdk-slow-tests
Commit ID: aa5d3b2
Result: FAILED
Build Logs (available for 30 days)

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

sagemaker-bot · 2023-02-16T19:25:45Z

AWS CodeBuild CI Report

CodeBuild project: sagemaker-python-sdk-slow-tests
Commit ID: aa5d3b2
Result: FAILED
Build Logs (available for 30 days)

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

jmahlik · 2023-02-17T16:18:11Z

I can rebase if needed. Whenever the flakey tests start passing, it'd be nice to fix this. It is kind of annoying supporting 200+ sagemaker users who keep running in to this. I've been trying since 2021. Sorry to say but I'll close this and refer them to support if we can't figure this out. Kind of hard to contribute here, maybe we can do a tad better. Seems to have started when the jumpstart tests went in.

claytonparnell · 2023-02-22T02:50:44Z

Apologies @jmahlik it has been merged. We've been trying to prioritize fixing the PR tests, will be resolved soon.

jmahlik mentioned this pull request Apr 6, 2022

local mode permission denied when file is written from running job to mounted volume #2647

Closed

BasilBeirouti closed this Aug 18, 2022

BasilBeirouti reopened this Aug 18, 2022

vikas0203 reviewed Oct 26, 2022

View reviewed changes

vikas0203 requested review from a team, navinsoni and vikas0203 and removed request for a team November 23, 2022 20:43

vikas0203 assigned navinsoni Nov 23, 2022

navinsoni changed the base branch from dev to master December 2, 2022 21:12

navinsoni suggested changes Dec 2, 2022

View reviewed changes

jmahlik force-pushed the fix-local-mode-temp-files branch from 2ea19d8 to 66fdd51 Compare December 14, 2022 04:19

jmahlik requested a review from a team as a code owner December 14, 2022 04:19

jmahlik requested review from navinsoni and vikas0203 and removed request for vikas0203 and navinsoni December 14, 2022 04:20

navinsoni reviewed Dec 15, 2022

View reviewed changes

navinsoni approved these changes Dec 15, 2022

View reviewed changes

mufaddal-rohawala force-pushed the master branch from bae3b8b to d000bdc Compare December 20, 2022 00:31

jmahlik force-pushed the fix-local-mode-temp-files branch from 6a3cea9 to fdeb5db Compare December 22, 2022 02:22

mufaddal-rohawala force-pushed the master branch from 8fbc1c6 to e6b8b15 Compare December 22, 2022 17:49

fix: local mode deletion of temp files on job end

aa5d3b2

navinsoni force-pushed the fix-local-mode-temp-files branch from fdeb5db to aa5d3b2 Compare January 4, 2023 01:51

claytonparnell reviewed Jan 18, 2023

View reviewed changes

trajanikant reviewed Feb 8, 2023

View reviewed changes

trajanikant assigned claytonparnell and unassigned navinsoni Feb 8, 2023

jmahlik requested a review from claytonparnell February 17, 2023 16:09

claytonparnell merged commit dcca7b7 into aws:master Feb 22, 2023

JoseJuan98 pushed a commit to JoseJuan98/sagemaker-python-sdk that referenced this pull request Mar 4, 2023

fix: local mode deletion of temp files on job end (aws#3017)

3bc0d07

nmadan pushed a commit to nmadan/sagemaker-python-sdk that referenced this pull request Apr 18, 2023

fix: local mode deletion of temp files on job end (aws#3017)

71a8380

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: local mode deletion of temp files on job end #3017

fix: local mode deletion of temp files on job end #3017

jmahlik commented Mar 22, 2022

codecov-commenter commented Mar 22, 2022 •

edited

Loading

sagemaker-bot commented Mar 22, 2022

sagemaker-bot commented Mar 22, 2022

sagemaker-bot commented Mar 22, 2022

vikas0203 left a comment

sagemaker-bot commented Oct 26, 2022

sagemaker-bot commented Oct 26, 2022

navinsoni left a comment

jmahlik commented Dec 6, 2022

jmahlik commented Dec 14, 2022

navinsoni left a comment

navinsoni commented Dec 15, 2022

sagemaker-bot commented Dec 19, 2022

sagemaker-bot commented Jan 5, 2023

sagemaker-bot commented Jan 5, 2023

sagemaker-bot commented Jan 5, 2023

sagemaker-bot commented Jan 5, 2023

sagemaker-bot commented Jan 6, 2023

jmahlik commented Jan 11, 2023

claytonparnell left a comment •

edited

Loading

claytonparnell commented Jan 18, 2023

sagemaker-bot commented Jan 18, 2023

sagemaker-bot commented Jan 18, 2023

trajanikant left a comment

sagemaker-bot commented Feb 8, 2023

sagemaker-bot commented Feb 8, 2023

sagemaker-bot commented Feb 10, 2023

sagemaker-bot commented Feb 16, 2023

sagemaker-bot commented Feb 16, 2023

jmahlik commented Feb 17, 2023

claytonparnell commented Feb 22, 2023

fix: local mode deletion of temp files on job end #3017

fix: local mode deletion of temp files on job end #3017

Conversation

jmahlik commented Mar 22, 2022

Merge Checklist

General

Tests

codecov-commenter commented Mar 22, 2022 • edited Loading

Codecov Report

sagemaker-bot commented Mar 22, 2022

AWS CodeBuild CI Report

sagemaker-bot commented Mar 22, 2022

AWS CodeBuild CI Report

sagemaker-bot commented Mar 22, 2022

AWS CodeBuild CI Report

vikas0203 left a comment

Choose a reason for hiding this comment

sagemaker-bot commented Oct 26, 2022

AWS CodeBuild CI Report

sagemaker-bot commented Oct 26, 2022

AWS CodeBuild CI Report

navinsoni left a comment

Choose a reason for hiding this comment

jmahlik commented Dec 6, 2022

jmahlik commented Dec 14, 2022

navinsoni left a comment

Choose a reason for hiding this comment

navinsoni commented Dec 15, 2022

sagemaker-bot commented Dec 19, 2022

AWS CodeBuild CI Report

sagemaker-bot commented Jan 5, 2023

AWS CodeBuild CI Report

sagemaker-bot commented Jan 5, 2023

AWS CodeBuild CI Report

sagemaker-bot commented Jan 5, 2023

AWS CodeBuild CI Report

sagemaker-bot commented Jan 5, 2023

AWS CodeBuild CI Report

sagemaker-bot commented Jan 6, 2023

AWS CodeBuild CI Report

jmahlik commented Jan 11, 2023

claytonparnell left a comment • edited Loading

Choose a reason for hiding this comment

claytonparnell commented Jan 18, 2023

sagemaker-bot commented Jan 18, 2023

AWS CodeBuild CI Report

sagemaker-bot commented Jan 18, 2023

AWS CodeBuild CI Report

trajanikant left a comment

Choose a reason for hiding this comment

sagemaker-bot commented Feb 8, 2023

AWS CodeBuild CI Report

sagemaker-bot commented Feb 8, 2023

AWS CodeBuild CI Report

sagemaker-bot commented Feb 10, 2023

AWS CodeBuild CI Report

sagemaker-bot commented Feb 16, 2023

AWS CodeBuild CI Report

sagemaker-bot commented Feb 16, 2023

AWS CodeBuild CI Report

jmahlik commented Feb 17, 2023

claytonparnell commented Feb 22, 2023

codecov-commenter commented Mar 22, 2022 •

edited

Loading

claytonparnell left a comment •

edited

Loading