breaking: create new inference resources during estimator.deploy() or estimator.transformer() by laurenyu · Pull Request #1639 · aws/sagemaker-python-sdk

laurenyu · 2020-06-26T16:54:35Z

Issue #, if available:
#1470, #987

Description of changes:
By reusing the training job name for models, endpoint configs, and endpoints, trying to call deploy() twice on the same estimator can lead to a frustrating experience (e.g. #987).

I thought about letting the respective Model classes handle the name generation, but thought it would be nice to honor base_job_name, so that's why I went with what I did.

Testing done:
unit and integ tests

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 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 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.

… estimator.transformer()

sagemaker-bot · 2020-06-26T16:57:07Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2020-06-26T17:20:54Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2020-06-26T17:56:40Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2020-06-26T17:59:25Z

AWS CodeBuild CI Report

CodeBuild project: sagemaker-python-sdk-zwei-pr
Commit ID: f0c10a5
Result: FAILED
Build Logs (available for 30 days)

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

sagemaker-bot · 2020-06-29T15:45:27Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2020-06-29T16:03:22Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2020-06-29T16:07:20Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2020-06-29T16:26:34Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2020-06-29T16:40:40Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2020-06-29T16:47:23Z

AWS CodeBuild CI Report

CodeBuild project: sagemaker-python-sdk-zwei-pr
Commit ID: 979348e
Result: FAILED
Build Logs (available for 30 days)

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

sagemaker-bot · 2020-06-29T17:02:19Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2020-06-29T17:42:48Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2020-06-29T18:01:55Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2020-06-29T18:06:14Z

AWS CodeBuild CI Report

CodeBuild project: sagemaker-python-sdk-zwei-pr
Commit ID: 7a52481
Result: FAILED
Build Logs (available for 30 days)

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

metrizable

looks good. a few questions, comments.

metrizable · 2020-06-29T18:01:14Z

src/sagemaker/estimator.py

+        if isinstance(self, sagemaker.algorithm.AlgorithmEstimator):
+            self.base_job_name = self.algorithm_arn.split("/")[-1]  # pylint: disable=no-member


Beyond scope for this PR, but I wonder if you want to model this so it can be resolved as an overridden method in AlgorithmEstimator to favor co-locating logic specific to derived classes to those derived classes. I've seen other violations of LSP/sub-type leakage elsewhere in the package, so it may be worth looking into in the future.

I'd been annoyed at this one previously, and hadn't changed it before because it was embedded in the method, but now that I'm refactoring it out...

metrizable · 2020-06-29T18:14:53Z

src/sagemaker/estimator.py

+        self._ensure_base_job_name()
+        model_name = model_name or name_from_base(self.base_job_name)


I've seen this logic elsewhere. Is it worth defining a method to capture and re-use?

created a method (feel free to suggest a better name for it)

metrizable · 2020-06-29T18:15:57Z

src/sagemaker/pytorch/estimator.py

+            self._ensure_base_job_name()
+            kwargs["name"] = utils.name_from_base(self.base_job_name)


I've seen this logic in multiple places. Is it worth creating a re-usable method?

I've changed it to use the method defined in response to your above comment. I know there's more in the create_model methods that could probably more generally be refactored, but I think that's out of scope here.

tests/integ/test_mxnet_train.py

sagemaker-bot · 2020-06-29T18:30:51Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2020-06-29T18:38:05Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2020-06-29T22:28:31Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2020-06-29T22:31:56Z

AWS CodeBuild CI Report

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

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

metrizable

looks good to me

sagemaker-bot · 2020-06-29T22:47:11Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2020-06-29T22:48:32Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2020-06-29T22:51:05Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2020-06-29T23:07:03Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2020-06-29T23:22:20Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2020-06-29T23:25:05Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2020-06-29T23:42:58Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2020-06-30T00:03:26Z

AWS CodeBuild CI Report

CodeBuild project: sagemaker-python-sdk-zwei-pr
Commit ID: f04ca11
Result: FAILED
Build Logs (available for 30 days)

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

sagemaker-bot · 2020-06-30T21:44:10Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2020-06-30T22:03:54Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2020-06-30T22:04:50Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2020-06-30T22:15:55Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2020-06-30T22:51:06Z

AWS CodeBuild CI Report

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

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

breaking: create new inference resources during estimator.deploy() or…

f0c10a5

… estimator.transformer()

laurenyu added 2 commits June 29, 2020 08:37

black, pylint

e03bf73

Merge branch 'zwei' into deploy-new-resources

ad3c102

fix unit test

979348e

laurenyu mentioned this pull request Jun 29, 2020

change: add deprecation warnings for estimator.delete_endpoint() and tuner.delete_endpoint() #1644

Merged

7 tasks

fix integ test and use list comprehension instead of map

7a52481

metrizable previously approved these changes Jun 29, 2020

View reviewed changes

laurenyu mentioned this pull request Jun 29, 2020

fix: ensure generated names are < 63 characters when deploying compiled models #1645

Merged

7 tasks

laurenyu added 2 commits June 29, 2020 14:43

address PR comments

8a400b5

Merge branch 'zwei' into deploy-new-resources

53cace5

laurenyu dismissed metrizable’s stale review via 53cace5 June 29, 2020 22:20

one more straggler

d03bca3

fix unit tests

f04ca11

metrizable approved these changes Jun 29, 2020

View reviewed changes

laurenyu mentioned this pull request Jun 30, 2020

breaking: deprecate delete_endpoint() for estimators and HyperparameterTuner #1651

Merged

7 tasks

Merge branch 'zwei' into deploy-new-resources

20d9faa

laurenyu merged commit d388519 into aws:zwei Jun 30, 2020

laurenyu deleted the deploy-new-resources branch June 30, 2020 22:54

pintaoz-aws pushed a commit that referenced this pull request Dec 4, 2024

Add model trainer documentation (#1639)

bd4a6cc

pintaoz-aws pushed a commit that referenced this pull request Dec 4, 2024

Add model trainer documentation (#1639)

854bafc

		if isinstance(self, sagemaker.algorithm.AlgorithmEstimator):
		self.base_job_name = self.algorithm_arn.split("/")[-1] # pylint: disable=no-member

		self._ensure_base_job_name()
		model_name = model_name or name_from_base(self.base_job_name)

		self._ensure_base_job_name()
		kwargs["name"] = utils.name_from_base(self.base_job_name)

Conversation

laurenyu commented Jun 26, 2020

Merge Checklist

General

Tests

Uh oh!

sagemaker-bot commented Jun 26, 2020

AWS CodeBuild CI Report

Uh oh!

sagemaker-bot commented Jun 26, 2020

AWS CodeBuild CI Report

Uh oh!

sagemaker-bot commented Jun 26, 2020

AWS CodeBuild CI Report

Uh oh!

sagemaker-bot commented Jun 26, 2020

AWS CodeBuild CI Report

Uh oh!

sagemaker-bot commented Jun 29, 2020

AWS CodeBuild CI Report

Uh oh!

sagemaker-bot commented Jun 29, 2020

AWS CodeBuild CI Report

Uh oh!

sagemaker-bot commented Jun 29, 2020

AWS CodeBuild CI Report

Uh oh!

sagemaker-bot commented Jun 29, 2020

AWS CodeBuild CI Report

Uh oh!

sagemaker-bot commented Jun 29, 2020

AWS CodeBuild CI Report

Uh oh!

sagemaker-bot commented Jun 29, 2020

AWS CodeBuild CI Report

Uh oh!

sagemaker-bot commented Jun 29, 2020

AWS CodeBuild CI Report

Uh oh!

sagemaker-bot commented Jun 29, 2020

AWS CodeBuild CI Report

Uh oh!

sagemaker-bot commented Jun 29, 2020

AWS CodeBuild CI Report

Uh oh!

sagemaker-bot commented Jun 29, 2020

AWS CodeBuild CI Report

Uh oh!

metrizable left a comment

Choose a reason for hiding this comment

Uh oh!

metrizable Jun 29, 2020

Choose a reason for hiding this comment

Uh oh!

laurenyu Jun 29, 2020

Choose a reason for hiding this comment

Uh oh!

metrizable Jun 29, 2020

Choose a reason for hiding this comment

Uh oh!

laurenyu Jun 29, 2020

Choose a reason for hiding this comment

Uh oh!

metrizable Jun 29, 2020

Choose a reason for hiding this comment

Uh oh!

laurenyu Jun 29, 2020

Choose a reason for hiding this comment

Uh oh!

Uh oh!

sagemaker-bot commented Jun 29, 2020

AWS CodeBuild CI Report

Uh oh!

sagemaker-bot commented Jun 29, 2020

AWS CodeBuild CI Report

Uh oh!

sagemaker-bot commented Jun 29, 2020

AWS CodeBuild CI Report

Uh oh!

sagemaker-bot commented Jun 29, 2020