From 6e09223c28463873de7bbca3cfb5a658fa45a297 Mon Sep 17 00:00:00 2001 From: Chuyang Deng Date: Wed, 13 Feb 2019 11:37:20 -0800 Subject: [PATCH 01/13] Support for Predictor to delete SageMaker model. --- README.rst | 8 ++++++++ src/sagemaker/predictor.py | 16 ++++++++++++++++ tests/integ/test_kmeans.py | 5 +++++ tests/integ/test_mxnet_train.py | 5 +++++ tests/unit/test_predictor.py | 15 +++++++++++++++ 5 files changed, 49 insertions(+) diff --git a/README.rst b/README.rst index a7ca3d748b..81688747e1 100644 --- a/README.rst +++ b/README.rst @@ -192,6 +192,8 @@ Here is an end to end example of how to use a SageMaker Estimator: # Tears down the SageMaker endpoint and endpoint configuration mxnet_predictor.delete_endpoint() + # Deletes SageMaker model + mxnet_predictor.delete_model() The example above will eventually delete both the SageMaker endpoint and endpoint configuration through `delete_endpoint()`. If you want to keep your SageMaker endpoint configuration, use the value False for the `delete_endpoint_config` parameter, as shown below. @@ -284,6 +286,9 @@ We can take the example in `Using Estimators <#using-estimators>`__ , and use e # Tears down the endpoint container and deletes the corresponding endpoint configuration mxnet_predictor.delete_endpoint() + # Deletes the model + mxnet_predictor.delete_model() + If you have an existing model and want to deploy it locally, don't specify a sagemaker_session argument to the ``MXNetModel`` constructor. The correct session is generated when you call ``model.deploy()``. @@ -307,6 +312,9 @@ Here is an end-to-end example: # Tear down the endpoint container and delete the corresponding endpoint configuration predictor.delete_endpoint() + # Deletes the model + predictor.delete_model() + If you don't want to deploy your model locally, you can also choose to perform a Local Batch Transform Job. This is useful if you want to test your container before creating a Sagemaker Batch Transform Job. Note that the performance diff --git a/src/sagemaker/predictor.py b/src/sagemaker/predictor.py index 958b21afe6..f4fd2ad85f 100644 --- a/src/sagemaker/predictor.py +++ b/src/sagemaker/predictor.py @@ -126,6 +126,22 @@ def delete_endpoint(self, delete_endpoint_config=True): self.sagemaker_session.delete_endpoint(self.endpoint) + def delete_model(self): + """Deletes the Amazon SageMaker models backing this predictor. + + """ + model_names = self._get_model_names() + for model_name in model_names: + self.sagemaker_session.delete_model(model_name) + + def _get_model_names(self): + endpoint_desc = self.sagemaker_session.sagemaker_client.describe_endpoint(EndpointName=self.endpoint) + endpoint_config_name = endpoint_desc['EndpointConfigName'] + endpoint_config = self.sagemaker_session.sagemaker_client.describe_endpoint_config(EndpointConfigName= + endpoint_config_name) + production_variants = endpoint_config['ProductionVariants'] + return map(lambda d: d['ModelName'], production_variants) + class _CsvSerializer(object): def __init__(self): diff --git a/tests/integ/test_kmeans.py b/tests/integ/test_kmeans.py index 85f6b247e8..37e18fbfd7 100644 --- a/tests/integ/test_kmeans.py +++ b/tests/integ/test_kmeans.py @@ -75,6 +75,11 @@ def test_kmeans(sagemaker_session): assert record.label["closest_cluster"] is not None assert record.label["distance_to_cluster"] is not None + predictor.delete_model() + with pytest.raises(Exception) as exception: + sagemaker_session.sagemaker_client.describe_model(ModelName=model.name) + assert 'Could not find model' in str(exception.value) + def test_async_kmeans(sagemaker_session): training_job_name = "" diff --git a/tests/integ/test_mxnet_train.py b/tests/integ/test_mxnet_train.py index 3d67f3b4c2..ed57789933 100644 --- a/tests/integ/test_mxnet_train.py +++ b/tests/integ/test_mxnet_train.py @@ -72,6 +72,11 @@ def test_deploy_model(mxnet_training_job, sagemaker_session, mxnet_full_version) data = numpy.zeros(shape=(1, 1, 28, 28)) predictor.predict(data) + predictor.delete_model() + with pytest.raises(Exception) as exception: + sagemaker_session.sagemaker_client.describe_model(ModelName=model.name) + assert 'Could not find model' in str(exception.value) + def test_deploy_model_with_update_endpoint(mxnet_training_job, sagemaker_session, mxnet_full_version): endpoint_name = 'test-mxnet-deploy-model-{}'.format(sagemaker_timestamp()) diff --git a/tests/unit/test_predictor.py b/tests/unit/test_predictor.py index 55ac1f5f9a..22106cf5a0 100644 --- a/tests/unit/test_predictor.py +++ b/tests/unit/test_predictor.py @@ -466,3 +466,18 @@ def test_delete_endpoint_only(): sagemaker_session.delete_endpoint.assert_called_with(ENDPOINT) sagemaker_session.delete_endpoint_config.assert_not_called() + + +def test_delete_model(): + endpoint_desc = { + 'ProductionVariants': [{ + 'VariantName': 'my-model' + }] + } + sagemaker_session = empty_sagemaker_session() + sagemaker_session.sagemaker_client.describe_endpoint = Mock(return_value=endpoint_desc) + predictor = RealTimePredictor(ENDPOINT, sagemaker_session=sagemaker_session) + + predictor.delete_model() + sagemaker_session.delete_model.assert_called_with('my-model') + From f38147580b151650c223928bc8f3251b621cd749 Mon Sep 17 00:00:00 2001 From: Chuyang Deng Date: Wed, 13 Feb 2019 11:52:00 -0800 Subject: [PATCH 02/13] Fix unit test. --- src/sagemaker/predictor.py | 4 ++-- tests/unit/test_predictor.py | 7 +++++-- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/src/sagemaker/predictor.py b/src/sagemaker/predictor.py index f4fd2ad85f..124acab39a 100644 --- a/src/sagemaker/predictor.py +++ b/src/sagemaker/predictor.py @@ -137,8 +137,8 @@ def delete_model(self): def _get_model_names(self): endpoint_desc = self.sagemaker_session.sagemaker_client.describe_endpoint(EndpointName=self.endpoint) endpoint_config_name = endpoint_desc['EndpointConfigName'] - endpoint_config = self.sagemaker_session.sagemaker_client.describe_endpoint_config(EndpointConfigName= - endpoint_config_name) + endpoint_config = self.sagemaker_session.sagemaker_client.describe_endpoint_config( + EndpointConfigName=endpoint_config_name) production_variants = endpoint_config['ProductionVariants'] return map(lambda d: d['ModelName'], production_variants) diff --git a/tests/unit/test_predictor.py b/tests/unit/test_predictor.py index 22106cf5a0..744390d20e 100644 --- a/tests/unit/test_predictor.py +++ b/tests/unit/test_predictor.py @@ -470,14 +470,17 @@ def test_delete_endpoint_only(): def test_delete_model(): endpoint_desc = { + 'EndpointConfigName': 'my-endpoint-config' + } + endpoint_config_desc = { 'ProductionVariants': [{ - 'VariantName': 'my-model' + 'ModelName': 'my-model' }] } sagemaker_session = empty_sagemaker_session() sagemaker_session.sagemaker_client.describe_endpoint = Mock(return_value=endpoint_desc) + sagemaker_session.sagemaker_client.describe_endpoint_config = Mock(return_value=endpoint_config_desc) predictor = RealTimePredictor(ENDPOINT, sagemaker_session=sagemaker_session) predictor.delete_model() sagemaker_session.delete_model.assert_called_with('my-model') - From 55193a2dce912ecb0e9f24dffcc00f7ecfcc94ce Mon Sep 17 00:00:00 2001 From: Chuyang Deng Date: Fri, 15 Feb 2019 10:11:33 -0800 Subject: [PATCH 03/13] Add delete_model to Predictor and Pipeline. --- src/sagemaker/pipeline.py | 11 ++++++++ src/sagemaker/predictor.py | 17 ++++++------- tests/integ/test_inference_pipeline.py | 5 ++++ tests/unit/test_chainer.py | 11 ++++++++ tests/unit/test_estimator.py | 11 ++++++++ tests/unit/test_fm.py | 11 ++++++++ tests/unit/test_ipinsights.py | 11 ++++++++ tests/unit/test_kmeans.py | 11 ++++++++ tests/unit/test_knn.py | 11 ++++++++ tests/unit/test_lda.py | 11 ++++++++ tests/unit/test_linear_learner.py | 11 ++++++++ tests/unit/test_mxnet.py | 11 ++++++++ tests/unit/test_ntm.py | 11 ++++++++ tests/unit/test_object2vec.py | 11 ++++++++ tests/unit/test_pca.py | 11 ++++++++ tests/unit/test_pipeline_model.py | 19 ++++++++++++++ tests/unit/test_predictor.py | 35 +++++++++++++++++--------- tests/unit/test_pytorch.py | 11 ++++++++ tests/unit/test_randomcutforest.py | 11 ++++++++ tests/unit/test_rl.py | 11 ++++++++ tests/unit/test_sklearn.py | 11 ++++++++ tests/unit/test_sparkml_serving.py | 11 ++++++++ tests/unit/test_tf_estimator.py | 11 ++++++++ tests/unit/test_tf_predictor.py | 11 ++++++++ tests/unit/test_tfs.py | 15 +++++++++-- tests/unit/test_tuner.py | 13 ++++++++++ 26 files changed, 301 insertions(+), 23 deletions(-) diff --git a/src/sagemaker/pipeline.py b/src/sagemaker/pipeline.py index af379a6b3d..4d9d3cd19e 100644 --- a/src/sagemaker/pipeline.py +++ b/src/sagemaker/pipeline.py @@ -103,3 +103,14 @@ def deploy(self, initial_instance_count, instance_type, endpoint_name=None, tags self.sagemaker_session.endpoint_from_production_variants(self.endpoint_name, [production_variant], tags) if self.predictor_cls: return self.predictor_cls(self.endpoint_name, self.sagemaker_session) + + def delete_model(self): + """Delete the SageMaker model backing this pipeline model. This does not delete the list of SageMaker models used + in multiple containers to build the inference pipeline. + + """ + + if self.name is None: + raise ValueError('The SageMaker model must be created before attempting to delete.') + + self.sagemaker_session.delete_model(self.name) diff --git a/src/sagemaker/predictor.py b/src/sagemaker/predictor.py index 124acab39a..790a1af3dd 100644 --- a/src/sagemaker/predictor.py +++ b/src/sagemaker/predictor.py @@ -56,6 +56,7 @@ def __init__(self, endpoint, sagemaker_session=None, serializer=None, deserializ self.deserializer = deserializer self.content_type = content_type or getattr(serializer, 'content_type', None) self.accept = accept or getattr(deserializer, 'accept', None) + self._model_names = self._get_model_names() def predict(self, data, initial_args=None): """Return the inference from the specified endpoint. @@ -109,16 +110,15 @@ def _delete_endpoint_config(self): """Delete the Amazon SageMaker endpoint configuration """ - endpoint_description = self.sagemaker_session.sagemaker_client.describe_endpoint(EndpointName=self.endpoint) - endpoint_config_name = endpoint_description['EndpointConfigName'] - self.sagemaker_session.delete_endpoint_config(endpoint_config_name) + self.sagemaker_session.delete_endpoint_config(self._endpoint_config_name) def delete_endpoint(self, delete_endpoint_config=True): """Delete the Amazon SageMaker endpoint and endpoint configuration backing this predictor. Args: - delete_endpoint_config (bool): Flag to indicate whether to delete the corresponding SageMaker endpoint - configuration tied to the endpoint. If False, only the endpoint will be deleted. (default: True) + delete_endpoint_config (bool, optional): Flag to indicate whether to delete endpoint configuration together + with endpoint. Defaults to True. If True, both endpoint and endpoint configuration will be deleted. If + False, only endpoint will be deleted. """ if delete_endpoint_config: @@ -130,15 +130,14 @@ def delete_model(self): """Deletes the Amazon SageMaker models backing this predictor. """ - model_names = self._get_model_names() - for model_name in model_names: + for model_name in self._model_names: self.sagemaker_session.delete_model(model_name) def _get_model_names(self): endpoint_desc = self.sagemaker_session.sagemaker_client.describe_endpoint(EndpointName=self.endpoint) - endpoint_config_name = endpoint_desc['EndpointConfigName'] + self._endpoint_config_name = endpoint_desc['EndpointConfigName'] endpoint_config = self.sagemaker_session.sagemaker_client.describe_endpoint_config( - EndpointConfigName=endpoint_config_name) + EndpointConfigName=self._endpoint_config_name) production_variants = endpoint_config['ProductionVariants'] return map(lambda d: d['ModelName'], production_variants) diff --git a/tests/integ/test_inference_pipeline.py b/tests/integ/test_inference_pipeline.py index d3c597b13e..1fbe45c618 100644 --- a/tests/integ/test_inference_pipeline.py +++ b/tests/integ/test_inference_pipeline.py @@ -92,3 +92,8 @@ def test_inference_pipeline_model_deploy(sagemaker_session): invalid_data = "1.0,28.0,C,38.0,71.5,1.0" assert (predictor.predict(invalid_data) is None) + + model.delete_model() + with pytest.raises(Exception) as exception: + sagemaker_session.sagemaker_client.describe_model(ModelName=model.name) + assert 'Could not find model' in str(exception.value) diff --git a/tests/unit/test_chainer.py b/tests/unit/test_chainer.py index fbc439ecde..aa712038da 100644 --- a/tests/unit/test_chainer.py +++ b/tests/unit/test_chainer.py @@ -45,6 +45,15 @@ GPU = 'ml.p2.xlarge' CPU = 'ml.c4.xlarge' +ENDPOINT_DESC = { + 'EndpointConfigName': 'test-endpoint' +} + +ENDPOINT_CONFIG_DESC = { + 'ProductionVariants': [{'ModelName': 'model-1'}, + {'ModelName': 'model-2'}] +} + @pytest.fixture() def sagemaker_session(): @@ -54,6 +63,8 @@ def sagemaker_session(): describe = {'ModelArtifacts': {'S3ModelArtifacts': 's3://m/m.tar.gz'}} session.sagemaker_client.describe_training_job = Mock(return_value=describe) + session.sagemaker_client.describe_endpoint = Mock(return_value=ENDPOINT_DESC) + session.sagemaker_client.describe_endpoint_config = Mock(return_value=ENDPOINT_CONFIG_DESC) session.default_bucket = Mock(name='default_bucket', return_value=BUCKET_NAME) session.expand_role = Mock(name="expand_role", return_value=ROLE) return session diff --git a/tests/unit/test_estimator.py b/tests/unit/test_estimator.py index 04afe65b5c..59d803254e 100644 --- a/tests/unit/test_estimator.py +++ b/tests/unit/test_estimator.py @@ -102,6 +102,15 @@ 'ModelDataUrl': MODEL_DATA, } +ENDPOINT_DESC = { + 'EndpointConfigName': 'test-endpoint' +} + +ENDPOINT_CONFIG_DESC = { + 'ProductionVariants': [{'ModelName': 'model-1'}, + {'ModelName': 'model-2'}] +} + class DummyFramework(Framework): __framework_name__ = 'dummy' @@ -146,6 +155,8 @@ def sagemaker_session(): sms.default_bucket = Mock(name='default_bucket', return_value=BUCKET_NAME) sms.sagemaker_client.describe_training_job = Mock(name='describe_training_job', return_value=DESCRIBE_TRAINING_JOB_RESULT) + sms.sagemaker_client.describe_endpoint = Mock(return_value=ENDPOINT_DESC) + sms.sagemaker_client.describe_endpoint_config = Mock(return_value=ENDPOINT_CONFIG_DESC) return sms diff --git a/tests/unit/test_fm.py b/tests/unit/test_fm.py index 9dbb7c014c..d420301378 100644 --- a/tests/unit/test_fm.py +++ b/tests/unit/test_fm.py @@ -37,6 +37,15 @@ } } +ENDPOINT_DESC = { + 'EndpointConfigName': 'test-endpoint' +} + +ENDPOINT_CONFIG_DESC = { + 'ProductionVariants': [{'ModelName': 'model-1'}, + {'ModelName': 'model-2'}] +} + @pytest.fixture() def sagemaker_session(): @@ -47,6 +56,8 @@ def sagemaker_session(): sms.default_bucket = Mock(name='default_bucket', return_value=BUCKET_NAME) sms.sagemaker_client.describe_training_job = Mock(name='describe_training_job', return_value=DESCRIBE_TRAINING_JOB_RESULT) + sms.sagemaker_client.describe_endpoint = Mock(return_value=ENDPOINT_DESC) + sms.sagemaker_client.describe_endpoint_config = Mock(return_value=ENDPOINT_CONFIG_DESC) return sms diff --git a/tests/unit/test_ipinsights.py b/tests/unit/test_ipinsights.py index e64adbd1be..402383348b 100644 --- a/tests/unit/test_ipinsights.py +++ b/tests/unit/test_ipinsights.py @@ -39,6 +39,15 @@ } } +ENDPOINT_DESC = { + 'EndpointConfigName': 'test-endpoint' +} + +ENDPOINT_CONFIG_DESC = { + 'ProductionVariants': [{'ModelName': 'model-1'}, + {'ModelName': 'model-2'}] +} + @pytest.fixture() def sagemaker_session(): @@ -49,6 +58,8 @@ def sagemaker_session(): sms.default_bucket = Mock(name='default_bucket', return_value=BUCKET_NAME) sms.sagemaker_client.describe_training_job = Mock(name='describe_training_job', return_value=DESCRIBE_TRAINING_JOB_RESULT) + sms.sagemaker_client.describe_endpoint = Mock(return_value=ENDPOINT_DESC) + sms.sagemaker_client.describe_endpoint_config = Mock(return_value=ENDPOINT_CONFIG_DESC) return sms diff --git a/tests/unit/test_kmeans.py b/tests/unit/test_kmeans.py index a5b829fa60..dc87a38ec1 100644 --- a/tests/unit/test_kmeans.py +++ b/tests/unit/test_kmeans.py @@ -36,6 +36,15 @@ } } +ENDPOINT_DESC = { + 'EndpointConfigName': 'test-endpoint' +} + +ENDPOINT_CONFIG_DESC = { + 'ProductionVariants': [{'ModelName': 'model-1'}, + {'ModelName': 'model-2'}] +} + @pytest.fixture() def sagemaker_session(): @@ -46,6 +55,8 @@ def sagemaker_session(): sms.default_bucket = Mock(name='default_bucket', return_value=BUCKET_NAME) sms.sagemaker_client.describe_training_job = Mock(name='describe_training_job', return_value=DESCRIBE_TRAINING_JOB_RESULT) + sms.sagemaker_client.describe_endpoint = Mock(return_value=ENDPOINT_DESC) + sms.sagemaker_client.describe_endpoint_config = Mock(return_value=ENDPOINT_CONFIG_DESC) return sms diff --git a/tests/unit/test_knn.py b/tests/unit/test_knn.py index 890f6ec9fc..fad5ae64b1 100644 --- a/tests/unit/test_knn.py +++ b/tests/unit/test_knn.py @@ -40,6 +40,15 @@ } } +ENDPOINT_DESC = { + 'EndpointConfigName': 'test-endpoint' +} + +ENDPOINT_CONFIG_DESC = { + 'ProductionVariants': [{'ModelName': 'model-1'}, + {'ModelName': 'model-2'}] +} + @pytest.fixture() def sagemaker_session(): @@ -50,6 +59,8 @@ def sagemaker_session(): sms.default_bucket = Mock(name='default_bucket', return_value=BUCKET_NAME) sms.sagemaker_client.describe_training_job = Mock(name='describe_training_job', return_value=DESCRIBE_TRAINING_JOB_RESULT) + sms.sagemaker_client.describe_endpoint = Mock(return_value=ENDPOINT_DESC) + sms.sagemaker_client.describe_endpoint_config = Mock(return_value=ENDPOINT_CONFIG_DESC) return sms diff --git a/tests/unit/test_lda.py b/tests/unit/test_lda.py index 5c0528e322..eed9902292 100644 --- a/tests/unit/test_lda.py +++ b/tests/unit/test_lda.py @@ -35,6 +35,15 @@ } } +ENDPOINT_DESC = { + 'EndpointConfigName': 'test-endpoint' +} + +ENDPOINT_CONFIG_DESC = { + 'ProductionVariants': [{'ModelName': 'model-1'}, + {'ModelName': 'model-2'}] +} + @pytest.fixture() def sagemaker_session(): @@ -44,6 +53,8 @@ def sagemaker_session(): sms.default_bucket = Mock(name='default_bucket', return_value=BUCKET_NAME) sms.sagemaker_client.describe_training_job = Mock(name='describe_training_job', return_value=DESCRIBE_TRAINING_JOB_RESULT) + sms.sagemaker_client.describe_endpoint = Mock(return_value=ENDPOINT_DESC) + sms.sagemaker_client.describe_endpoint_config = Mock(return_value=ENDPOINT_CONFIG_DESC) return sms diff --git a/tests/unit/test_linear_learner.py b/tests/unit/test_linear_learner.py index 8819717957..1e5fa64f0f 100644 --- a/tests/unit/test_linear_learner.py +++ b/tests/unit/test_linear_learner.py @@ -37,6 +37,15 @@ } } +ENDPOINT_DESC = { + 'EndpointConfigName': 'test-endpoint' +} + +ENDPOINT_CONFIG_DESC = { + 'ProductionVariants': [{'ModelName': 'model-1'}, + {'ModelName': 'model-2'}] +} + @pytest.fixture() def sagemaker_session(): @@ -47,6 +56,8 @@ def sagemaker_session(): sms.default_bucket = Mock(name='default_bucket', return_value=BUCKET_NAME) sms.sagemaker_client.describe_training_job = Mock(name='describe_training_job', return_value=DESCRIBE_TRAINING_JOB_RESULT) + sms.sagemaker_client.describe_endpoint = Mock(return_value=ENDPOINT_DESC) + sms.sagemaker_client.describe_endpoint_config = Mock(return_value=ENDPOINT_CONFIG_DESC) return sms diff --git a/tests/unit/test_mxnet.py b/tests/unit/test_mxnet.py index 0136053c13..18302fdb20 100644 --- a/tests/unit/test_mxnet.py +++ b/tests/unit/test_mxnet.py @@ -45,6 +45,15 @@ CPU_C5 = 'ml.c5.xlarge' LAUNCH_PS_DISTRIBUTIONS_DICT = {'parameter_server': {'enabled': True}} +ENDPOINT_DESC = { + 'EndpointConfigName': 'test-endpoint' +} + +ENDPOINT_CONFIG_DESC = { + 'ProductionVariants': [{'ModelName': 'model-1'}, + {'ModelName': 'model-2'}] +} + @pytest.fixture() def sagemaker_session(): @@ -55,6 +64,8 @@ def sagemaker_session(): describe = {'ModelArtifacts': {'S3ModelArtifacts': 's3://m/m.tar.gz'}} describe_compilation = {'ModelArtifacts': {'S3ModelArtifacts': 's3://m/model_c5.tar.gz'}} session.sagemaker_client.describe_training_job = Mock(return_value=describe) + session.sagemaker_client.describe_endpoint = Mock(return_value=ENDPOINT_DESC) + session.sagemaker_client.describe_endpoint_config = Mock(return_value=ENDPOINT_CONFIG_DESC) session.wait_for_compilation_job = Mock(return_value=describe_compilation) session.default_bucket = Mock(name='default_bucket', return_value=BUCKET_NAME) session.expand_role = Mock(name="expand_role", return_value=ROLE) diff --git a/tests/unit/test_ntm.py b/tests/unit/test_ntm.py index 1f3866968b..c72e8cea70 100644 --- a/tests/unit/test_ntm.py +++ b/tests/unit/test_ntm.py @@ -36,6 +36,15 @@ } } +ENDPOINT_DESC = { + 'EndpointConfigName': 'test-endpoint' +} + +ENDPOINT_CONFIG_DESC = { + 'ProductionVariants': [{'ModelName': 'model-1'}, + {'ModelName': 'model-2'}] +} + @pytest.fixture() def sagemaker_session(): @@ -46,6 +55,8 @@ def sagemaker_session(): sms.default_bucket = Mock(name='default_bucket', return_value=BUCKET_NAME) sms.sagemaker_client.describe_training_job = Mock(name='describe_training_job', return_value=DESCRIBE_TRAINING_JOB_RESULT) + sms.sagemaker_client.describe_endpoint = Mock(return_value=ENDPOINT_DESC) + sms.sagemaker_client.describe_endpoint_config = Mock(return_value=ENDPOINT_CONFIG_DESC) return sms diff --git a/tests/unit/test_object2vec.py b/tests/unit/test_object2vec.py index 4fc6130722..8d8e557014 100644 --- a/tests/unit/test_object2vec.py +++ b/tests/unit/test_object2vec.py @@ -45,6 +45,15 @@ } } +ENDPOINT_DESC = { + 'EndpointConfigName': 'test-endpoint' +} + +ENDPOINT_CONFIG_DESC = { + 'ProductionVariants': [{'ModelName': 'model-1'}, + {'ModelName': 'model-2'}] +} + @pytest.fixture() def sagemaker_session(): @@ -55,6 +64,8 @@ def sagemaker_session(): sms.default_bucket = Mock(name='default_bucket', return_value=BUCKET_NAME) sms.sagemaker_client.describe_training_job = Mock(name='describe_training_job', return_value=DESCRIBE_TRAINING_JOB_RESULT) + sms.sagemaker_client.describe_endpoint = Mock(return_value=ENDPOINT_DESC) + sms.sagemaker_client.describe_endpoint_config = Mock(return_value=ENDPOINT_CONFIG_DESC) return sms diff --git a/tests/unit/test_pca.py b/tests/unit/test_pca.py index c72cd4fedd..0748d90ca6 100644 --- a/tests/unit/test_pca.py +++ b/tests/unit/test_pca.py @@ -36,6 +36,15 @@ } } +ENDPOINT_DESC = { + 'EndpointConfigName': 'test-endpoint' +} + +ENDPOINT_CONFIG_DESC = { + 'ProductionVariants': [{'ModelName': 'model-1'}, + {'ModelName': 'model-2'}] +} + @pytest.fixture() def sagemaker_session(): @@ -46,6 +55,8 @@ def sagemaker_session(): sms.default_bucket = Mock(name='default_bucket', return_value=BUCKET_NAME) sms.sagemaker_client.describe_training_job = Mock(name='describe_training_job', return_value=DESCRIBE_TRAINING_JOB_RESULT) + sms.sagemaker_client.describe_endpoint = Mock(return_value=ENDPOINT_DESC) + sms.sagemaker_client.describe_endpoint_config = Mock(return_value=ENDPOINT_CONFIG_DESC) return sms diff --git a/tests/unit/test_pipeline_model.py b/tests/unit/test_pipeline_model.py index e45c34409e..d640aa3235 100644 --- a/tests/unit/test_pipeline_model.py +++ b/tests/unit/test_pipeline_model.py @@ -138,3 +138,22 @@ def test_deploy_tags(tfo, time, sagemaker_session): 'InitialInstanceCount': 1, 'VariantName': 'AllTraffic'}], tags) + + +def test_delete_model_without_deploy(sagemaker_session): + pipeline_model = PipelineModel([], role=ROLE, sagemaker_session=sagemaker_session) + + expected_error_message = 'The SageMaker model must be created before attempting to delete.' + with pytest.raises(ValueError, match=expected_error_message): + pipeline_model.delete_model() + + +@patch('tarfile.open') +@patch('time.strftime', return_value=TIMESTAMP) +def test_delete_model(tfo, time, sagemaker_session): + framework_model = DummyFrameworkModel(sagemaker_session) + pipeline_model = PipelineModel([framework_model], role=ROLE, sagemaker_session=sagemaker_session) + pipeline_model.deploy(instance_type=INSTANCE_TYPE, initial_instance_count=1) + + pipeline_model.delete_model() + sagemaker_session.delete_model.assert_called_with(pipeline_model.name) diff --git a/tests/unit/test_predictor.py b/tests/unit/test_predictor.py index 744390d20e..c03b3a338f 100644 --- a/tests/unit/test_predictor.py +++ b/tests/unit/test_predictor.py @@ -16,7 +16,7 @@ import json import os import pytest -from mock import Mock +from mock import Mock, call import numpy as np @@ -319,11 +319,22 @@ def test_numpy_deser_from_npy_object_array(): RETURN_VALUE = 0 CSV_RETURN_VALUE = "1,2,3\r\n" +ENDPOINT_DESC = { + 'EndpointConfigName': ENDPOINT +} + +ENDPOINT_CONFIG_DESC = { + 'ProductionVariants': [{'ModelName': 'model-1'}, + {'ModelName': 'model-2'}] +} + def empty_sagemaker_session(): ims = Mock(name='sagemaker_session') ims.default_bucket = Mock(name='default_bucket', return_value=BUCKET_NAME) ims.sagemaker_runtime_client = Mock(name='sagemaker_runtime') + ims.sagemaker_client.describe_endpoint = Mock(return_value=ENDPOINT_DESC) + ims.sagemaker_client.describe_endpoint_config = Mock(return_value=ENDPOINT_CONFIG_DESC) response_body = Mock('body') response_body.read = Mock('read', return_value=RETURN_VALUE) @@ -379,6 +390,9 @@ def json_sagemaker_session(): ims.default_bucket = Mock(name='default_bucket', return_value=BUCKET_NAME) ims.sagemaker_runtime_client = Mock(name='sagemaker_runtime') + ims.sagemaker_client.describe_endpoint = Mock(return_value=ENDPOINT_DESC) + ims.sagemaker_client.describe_endpoint_config = Mock(return_value=ENDPOINT_CONFIG_DESC) + response_body = Mock('body') response_body.read = Mock('read', return_value=json.dumps([RETURN_VALUE])) response_body.close = Mock('close', return_value=None) @@ -417,6 +431,9 @@ def ret_csv_sagemaker_session(): ims.default_bucket = Mock(name='default_bucket', return_value=BUCKET_NAME) ims.sagemaker_runtime_client = Mock(name='sagemaker_runtime') + ims.sagemaker_client.describe_endpoint = Mock(return_value=ENDPOINT_DESC) + ims.sagemaker_client.describe_endpoint_config = Mock(return_value=ENDPOINT_CONFIG_DESC) + response_body = Mock('body') response_body.read = Mock('read', return_value=CSV_RETURN_VALUE) response_body.close = Mock('close', return_value=None) @@ -469,18 +486,12 @@ def test_delete_endpoint_only(): def test_delete_model(): - endpoint_desc = { - 'EndpointConfigName': 'my-endpoint-config' - } - endpoint_config_desc = { - 'ProductionVariants': [{ - 'ModelName': 'my-model' - }] - } sagemaker_session = empty_sagemaker_session() - sagemaker_session.sagemaker_client.describe_endpoint = Mock(return_value=endpoint_desc) - sagemaker_session.sagemaker_client.describe_endpoint_config = Mock(return_value=endpoint_config_desc) predictor = RealTimePredictor(ENDPOINT, sagemaker_session=sagemaker_session) predictor.delete_model() - sagemaker_session.delete_model.assert_called_with('my-model') + + expected_call_count = 2 + expected_call_args_list = [call('model-1'), call('model-2')] + assert sagemaker_session.delete_model.call_count == expected_call_count + assert sagemaker_session.delete_model.call_args_list == expected_call_args_list diff --git a/tests/unit/test_pytorch.py b/tests/unit/test_pytorch.py index 20edf62640..3a71d320b8 100644 --- a/tests/unit/test_pytorch.py +++ b/tests/unit/test_pytorch.py @@ -43,6 +43,15 @@ GPU = 'ml.p2.xlarge' CPU = 'ml.c4.xlarge' +ENDPOINT_DESC = { + 'EndpointConfigName': 'test-endpoint' +} + +ENDPOINT_CONFIG_DESC = { + 'ProductionVariants': [{'ModelName': 'model-1'}, + {'ModelName': 'model-2'}] +} + @pytest.fixture(name='sagemaker_session') def fixture_sagemaker_session(): @@ -52,6 +61,8 @@ def fixture_sagemaker_session(): describe = {'ModelArtifacts': {'S3ModelArtifacts': 's3://m/m.tar.gz'}} session.sagemaker_client.describe_training_job = Mock(return_value=describe) + session.sagemaker_client.describe_endpoint = Mock(return_value=ENDPOINT_DESC) + session.sagemaker_client.describe_endpoint_config = Mock(return_value=ENDPOINT_CONFIG_DESC) session.default_bucket = Mock(name='default_bucket', return_value=BUCKET_NAME) session.expand_role = Mock(name="expand_role", return_value=ROLE) return session diff --git a/tests/unit/test_randomcutforest.py b/tests/unit/test_randomcutforest.py index d265d22ba6..561f7c29ac 100644 --- a/tests/unit/test_randomcutforest.py +++ b/tests/unit/test_randomcutforest.py @@ -38,6 +38,15 @@ } } +ENDPOINT_DESC = { + 'EndpointConfigName': 'test-endpoint' +} + +ENDPOINT_CONFIG_DESC = { + 'ProductionVariants': [{'ModelName': 'model-1'}, + {'ModelName': 'model-2'}] +} + @pytest.fixture() def sagemaker_session(): @@ -48,6 +57,8 @@ def sagemaker_session(): sms.default_bucket = Mock(name='default_bucket', return_value=BUCKET_NAME) sms.sagemaker_client.describe_training_job = Mock(name='describe_training_job', return_value=DESCRIBE_TRAINING_JOB_RESULT) + sms.sagemaker_client.describe_endpoint = Mock(return_value=ENDPOINT_DESC) + sms.sagemaker_client.describe_endpoint_config = Mock(return_value=ENDPOINT_CONFIG_DESC) return sms diff --git a/tests/unit/test_rl.py b/tests/unit/test_rl.py index c9e28f3c23..a4657c901d 100644 --- a/tests/unit/test_rl.py +++ b/tests/unit/test_rl.py @@ -40,6 +40,15 @@ GPU = 'ml.p2.xlarge' CPU = 'ml.c4.xlarge' +ENDPOINT_DESC = { + 'EndpointConfigName': 'test-endpoint' +} + +ENDPOINT_CONFIG_DESC = { + 'ProductionVariants': [{'ModelName': 'model-1'}, + {'ModelName': 'model-2'}] +} + @pytest.fixture(name='sagemaker_session') def fixture_sagemaker_session(): @@ -49,6 +58,8 @@ def fixture_sagemaker_session(): describe = {'ModelArtifacts': {'S3ModelArtifacts': 's3://m/m.tar.gz'}} session.sagemaker_client.describe_training_job = Mock(return_value=describe) + session.sagemaker_client.describe_endpoint = Mock(return_value=ENDPOINT_DESC) + session.sagemaker_client.describe_endpoint_config = Mock(return_value=ENDPOINT_CONFIG_DESC) session.default_bucket = Mock(name='default_bucket', return_value=BUCKET_NAME) session.expand_role = Mock(name="expand_role", return_value=ROLE) return session diff --git a/tests/unit/test_sklearn.py b/tests/unit/test_sklearn.py index 5795aa3e90..9a3b199156 100644 --- a/tests/unit/test_sklearn.py +++ b/tests/unit/test_sklearn.py @@ -43,6 +43,15 @@ REGION = 'us-west-2' CPU = 'ml.c4.xlarge' +ENDPOINT_DESC = { + 'EndpointConfigName': 'test-endpoint' +} + +ENDPOINT_CONFIG_DESC = { + 'ProductionVariants': [{'ModelName': 'model-1'}, + {'ModelName': 'model-2'}] +} + @pytest.fixture() def sagemaker_session(): @@ -52,6 +61,8 @@ def sagemaker_session(): describe = {'ModelArtifacts': {'S3ModelArtifacts': 's3://m/m.tar.gz'}} session.sagemaker_client.describe_training_job = Mock(return_value=describe) + session.sagemaker_client.describe_endpoint = Mock(return_value=ENDPOINT_DESC) + session.sagemaker_client.describe_endpoint_config = Mock(return_value=ENDPOINT_CONFIG_DESC) session.default_bucket = Mock(name='default_bucket', return_value=BUCKET_NAME) session.expand_role = Mock(name="expand_role", return_value=ROLE) return session diff --git a/tests/unit/test_sparkml_serving.py b/tests/unit/test_sparkml_serving.py index bbd588170d..bacf64a81f 100644 --- a/tests/unit/test_sparkml_serving.py +++ b/tests/unit/test_sparkml_serving.py @@ -26,6 +26,15 @@ BUCKET_NAME = 'Some-Bucket' ENDPOINT = 'some-endpoint' +ENDPOINT_DESC = { + 'EndpointConfigName': ENDPOINT +} + +ENDPOINT_CONFIG_DESC = { + 'ProductionVariants': [{'ModelName': 'model-1'}, + {'ModelName': 'model-2'}] +} + @pytest.fixture() def sagemaker_session(): @@ -33,6 +42,8 @@ def sagemaker_session(): sms = Mock(name='sagemaker_session', boto_session=boto_mock, region_name=REGION, config=None, local_mode=False) sms.boto_region_name = REGION + sms.sagemaker_client.describe_endpoint = Mock(return_value=ENDPOINT_DESC) + sms.sagemaker_client.describe_endpoint_config = Mock(return_value=ENDPOINT_CONFIG_DESC) return sms diff --git a/tests/unit/test_tf_estimator.py b/tests/unit/test_tf_estimator.py index d6482ae479..e2777d0326 100644 --- a/tests/unit/test_tf_estimator.py +++ b/tests/unit/test_tf_estimator.py @@ -48,6 +48,15 @@ DISTRIBUTION_ENABLED = {'parameter_server': {'enabled': True}} DISTRIBUTION_MPI_ENABLED = {'mpi': {'enabled': True, 'custom_mpi_options': 'options', 'processes_per_host': 2}} +ENDPOINT_DESC = { + 'EndpointConfigName': 'test-endpoint' +} + +ENDPOINT_CONFIG_DESC = { + 'ProductionVariants': [{'ModelName': 'model-1'}, + {'ModelName': 'model-2'}] +} + @pytest.fixture() def sagemaker_session(): @@ -58,6 +67,8 @@ def sagemaker_session(): session.expand_role = Mock(name="expand_role", return_value=ROLE) describe = {'ModelArtifacts': {'S3ModelArtifacts': 's3://m/m.tar.gz'}} session.sagemaker_client.describe_training_job = Mock(return_value=describe) + session.sagemaker_client.describe_endpoint = Mock(return_value=ENDPOINT_DESC) + session.sagemaker_client.describe_endpoint_config = Mock(return_value=ENDPOINT_CONFIG_DESC) return session diff --git a/tests/unit/test_tf_predictor.py b/tests/unit/test_tf_predictor.py index a9ff684df6..b58971adbf 100644 --- a/tests/unit/test_tf_predictor.py +++ b/tests/unit/test_tf_predictor.py @@ -43,12 +43,23 @@ JSON_CONTENT_TYPE = "application/json" PROTO_CONTENT_TYPE = "application/octet-stream" +ENDPOINT_DESC = { + 'EndpointConfigName': ENDPOINT +} + +ENDPOINT_CONFIG_DESC = { + 'ProductionVariants': [{'ModelName': 'model-1'}, + {'ModelName': 'model-2'}] +} + @pytest.fixture() def sagemaker_session(): boto_mock = Mock(name='boto_session', region_name=REGION) ims = Mock(name='sagemaker_session', boto_session=boto_mock) ims.default_bucket = Mock(name='default_bucket', return_value=BUCKET_NAME) + ims.sagemaker_client.describe_endpoint = Mock(return_value=ENDPOINT_DESC) + ims.sagemaker_client.describe_endpoint_config = Mock(return_value=ENDPOINT_CONFIG_DESC) return ims diff --git a/tests/unit/test_tfs.py b/tests/unit/test_tfs.py index e29e3d4098..77f68561a2 100644 --- a/tests/unit/test_tfs.py +++ b/tests/unit/test_tfs.py @@ -40,6 +40,15 @@ } REGRESS_RESPONSE = {'results': [3.5, 4.0]} +ENDPOINT_DESC = { + 'EndpointConfigName': 'test-endpoint' +} + +ENDPOINT_CONFIG_DESC = { + 'ProductionVariants': [{'ModelName': 'model-1'}, + {'ModelName': 'model-2'}] +} + @pytest.fixture() def sagemaker_session(): @@ -50,6 +59,8 @@ def sagemaker_session(): session.expand_role = Mock(name="expand_role", return_value=ROLE) describe = {'ModelArtifacts': {'S3ModelArtifacts': 's3://m/m.tar.gz'}} session.sagemaker_client.describe_training_job = Mock(return_value=describe) + session.sagemaker_client.describe_endpoint = Mock(return_value=ENDPOINT_DESC) + session.sagemaker_client.describe_endpoint_config = Mock(return_value=ENDPOINT_CONFIG_DESC) return session @@ -191,14 +202,14 @@ def test_predictor_regress(sagemaker_session): assert REGRESS_RESPONSE == result -def test_predictor_regress_bad_content_type(): +def test_predictor_regress_bad_content_type(sagemaker_session): predictor = Predictor('endpoint', sagemaker_session, csv_serializer) with pytest.raises(ValueError): predictor.regress(REGRESS_INPUT) -def test_predictor_classify_bad_content_type(): +def test_predictor_classify_bad_content_type(sagemaker_session): predictor = Predictor('endpoint', sagemaker_session, csv_serializer) with pytest.raises(ValueError): diff --git a/tests/unit/test_tuner.py b/tests/unit/test_tuner.py index 76480d9cf5..4fec84965e 100644 --- a/tests/unit/test_tuner.py +++ b/tests/unit/test_tuner.py @@ -127,6 +127,15 @@ 'HyperParameterTuningJobArn': 'arn:tuning_job', } +ENDPOINT_DESC = { + 'EndpointConfigName': 'test-endpoint' +} + +ENDPOINT_CONFIG_DESC = { + 'ProductionVariants': [{'ModelName': 'model-1'}, + {'ModelName': 'model-2'}] +} + @pytest.fixture() def sagemaker_session(): @@ -135,6 +144,10 @@ def sagemaker_session(): sms.boto_region_name = REGION sms.default_bucket = Mock(name='default_bucket', return_value=BUCKET_NAME) sms.config = None + + sms.sagemaker_client.describe_endpoint = Mock(return_value=ENDPOINT_DESC) + sms.sagemaker_client.describe_endpoint_config = Mock(return_value=ENDPOINT_CONFIG_DESC) + return sms From eae499c4949886bb12f5af573293362aa395c9f6 Mon Sep 17 00:00:00 2001 From: Chuyang Deng Date: Fri, 15 Feb 2019 11:50:53 -0800 Subject: [PATCH 04/13] initialize endpoint config and model names within predictor --- CHANGELOG.rst | 2 ++ src/sagemaker/predictor.py | 6 +++++- 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index fada8fd8ef..a7757cad0f 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -6,6 +6,8 @@ CHANGELOG ========== * doc-fix: Remove incorrect parameter for EI TFS Python README +* feature: ``Predictor``: delete SageMaker model +* feature: ``Pipeline``: delete SageMaker model 1.18.3.post1 ============ diff --git a/src/sagemaker/predictor.py b/src/sagemaker/predictor.py index 790a1af3dd..b2473ebf98 100644 --- a/src/sagemaker/predictor.py +++ b/src/sagemaker/predictor.py @@ -133,11 +133,15 @@ def delete_model(self): for model_name in self._model_names: self.sagemaker_session.delete_model(model_name) - def _get_model_names(self): + def _get_endpoint_config_desc(self): endpoint_desc = self.sagemaker_session.sagemaker_client.describe_endpoint(EndpointName=self.endpoint) self._endpoint_config_name = endpoint_desc['EndpointConfigName'] endpoint_config = self.sagemaker_session.sagemaker_client.describe_endpoint_config( EndpointConfigName=self._endpoint_config_name) + return endpoint_config + + def _get_model_names(self): + endpoint_config = self._get_endpoint_config_desc() production_variants = endpoint_config['ProductionVariants'] return map(lambda d: d['ModelName'], production_variants) From c055924fd8cdfdbeeccb08bf86372b73e992be4a Mon Sep 17 00:00:00 2001 From: Chuyang Deng Date: Fri, 15 Feb 2019 13:38:04 -0800 Subject: [PATCH 05/13] Add a missed README example change. --- README.rst | 3 +++ 1 file changed, 3 insertions(+) diff --git a/README.rst b/README.rst index 81688747e1..aa509ea301 100644 --- a/README.rst +++ b/README.rst @@ -232,6 +232,9 @@ For more `information Date: Fri, 15 Feb 2019 14:46:11 -0800 Subject: [PATCH 06/13] update docs and test for predictor --- src/sagemaker/predictor.py | 3 ++- tests/unit/test_predictor.py | 4 ++++ 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/src/sagemaker/predictor.py b/src/sagemaker/predictor.py index b2473ebf98..9456f77d32 100644 --- a/src/sagemaker/predictor.py +++ b/src/sagemaker/predictor.py @@ -113,7 +113,8 @@ def _delete_endpoint_config(self): self.sagemaker_session.delete_endpoint_config(self._endpoint_config_name) def delete_endpoint(self, delete_endpoint_config=True): - """Delete the Amazon SageMaker endpoint and endpoint configuration backing this predictor. + """Delete the Amazon SageMaker endpoint backing this predictor. Also delete the endpoint configuration attached + to it if delete_endpoint_config is True. Args: delete_endpoint_config (bool, optional): Flag to indicate whether to delete endpoint configuration together diff --git a/tests/unit/test_predictor.py b/tests/unit/test_predictor.py index c03b3a338f..66b3b5cf73 100644 --- a/tests/unit/test_predictor.py +++ b/tests/unit/test_predictor.py @@ -389,6 +389,8 @@ def json_sagemaker_session(): ims = Mock(name='sagemaker_session') ims.default_bucket = Mock(name='default_bucket', return_value=BUCKET_NAME) ims.sagemaker_runtime_client = Mock(name='sagemaker_runtime') + ims.sagemaker_client.describe_endpoint = Mock(return_value=ENDPOINT_DESC) + ims.sagemaker_client.describe_endpoint_config = Mock(return_value=ENDPOINT_CONFIG_DESC) ims.sagemaker_client.describe_endpoint = Mock(return_value=ENDPOINT_DESC) ims.sagemaker_client.describe_endpoint_config = Mock(return_value=ENDPOINT_CONFIG_DESC) @@ -430,6 +432,8 @@ def ret_csv_sagemaker_session(): ims = Mock(name='sagemaker_session') ims.default_bucket = Mock(name='default_bucket', return_value=BUCKET_NAME) ims.sagemaker_runtime_client = Mock(name='sagemaker_runtime') + ims.sagemaker_client.describe_endpoint = Mock(return_value=ENDPOINT_DESC) + ims.sagemaker_client.describe_endpoint_config = Mock(return_value=ENDPOINT_CONFIG_DESC) ims.sagemaker_client.describe_endpoint = Mock(return_value=ENDPOINT_DESC) ims.sagemaker_client.describe_endpoint_config = Mock(return_value=ENDPOINT_CONFIG_DESC) From edd00a67a992a674361aac314ed3e9ebb01392cf Mon Sep 17 00:00:00 2001 From: Chuyang Deng Date: Mon, 18 Feb 2019 10:28:06 -0800 Subject: [PATCH 07/13] Init endpoint_config_name in Predictor's constructor. --- src/sagemaker/predictor.py | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/sagemaker/predictor.py b/src/sagemaker/predictor.py index 9456f77d32..4838f67c04 100644 --- a/src/sagemaker/predictor.py +++ b/src/sagemaker/predictor.py @@ -56,7 +56,8 @@ def __init__(self, endpoint, sagemaker_session=None, serializer=None, deserializ self.deserializer = deserializer self.content_type = content_type or getattr(serializer, 'content_type', None) self.accept = accept or getattr(deserializer, 'accept', None) - self._model_names = self._get_model_names() + self._endpoint_config_name = self._get_endpoint_config_name() + self._model_names = self._endpoint_config_desc_and_model_names() def predict(self, data, initial_args=None): """Return the inference from the specified endpoint. @@ -134,15 +135,14 @@ def delete_model(self): for model_name in self._model_names: self.sagemaker_session.delete_model(model_name) - def _get_endpoint_config_desc(self): + def _get_endpoint_config_name(self): endpoint_desc = self.sagemaker_session.sagemaker_client.describe_endpoint(EndpointName=self.endpoint) - self._endpoint_config_name = endpoint_desc['EndpointConfigName'] + endpoint_config_name = endpoint_desc['EndpointConfigName'] + return endpoint_config_name + + def _endpoint_config_desc_and_model_names(self): endpoint_config = self.sagemaker_session.sagemaker_client.describe_endpoint_config( EndpointConfigName=self._endpoint_config_name) - return endpoint_config - - def _get_model_names(self): - endpoint_config = self._get_endpoint_config_desc() production_variants = endpoint_config['ProductionVariants'] return map(lambda d: d['ModelName'], production_variants) From c89b8f514db497d6aca0e49231dcfd144a5ab195 Mon Sep 17 00:00:00 2001 From: Chuyang Deng Date: Mon, 18 Feb 2019 11:32:37 -0800 Subject: [PATCH 08/13] Modify method name (_get_model_names). --- src/sagemaker/predictor.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/sagemaker/predictor.py b/src/sagemaker/predictor.py index 4838f67c04..7f9f46c2fe 100644 --- a/src/sagemaker/predictor.py +++ b/src/sagemaker/predictor.py @@ -57,7 +57,7 @@ def __init__(self, endpoint, sagemaker_session=None, serializer=None, deserializ self.content_type = content_type or getattr(serializer, 'content_type', None) self.accept = accept or getattr(deserializer, 'accept', None) self._endpoint_config_name = self._get_endpoint_config_name() - self._model_names = self._endpoint_config_desc_and_model_names() + self._model_names = self._get_model_names() def predict(self, data, initial_args=None): """Return the inference from the specified endpoint. @@ -140,7 +140,7 @@ def _get_endpoint_config_name(self): endpoint_config_name = endpoint_desc['EndpointConfigName'] return endpoint_config_name - def _endpoint_config_desc_and_model_names(self): + def _get_model_names(self): endpoint_config = self.sagemaker_session.sagemaker_client.describe_endpoint_config( EndpointConfigName=self._endpoint_config_name) production_variants = endpoint_config['ProductionVariants'] From c552c049ff880d2df731e6f3eac4583c5627a3c0 Mon Sep 17 00:00:00 2001 From: Chuyang Deng Date: Tue, 19 Feb 2019 11:04:11 -0800 Subject: [PATCH 09/13] Update README docstring and predictor delete model error handling. --- README.rst | 4 ++-- src/sagemaker/predictor.py | 7 +++++-- tests/unit/test_predictor.py | 12 ++++++++++++ 3 files changed, 19 insertions(+), 4 deletions(-) diff --git a/README.rst b/README.rst index aa509ea301..682fc60a4f 100644 --- a/README.rst +++ b/README.rst @@ -192,7 +192,7 @@ Here is an end to end example of how to use a SageMaker Estimator: # Tears down the SageMaker endpoint and endpoint configuration mxnet_predictor.delete_endpoint() - # Deletes SageMaker model + # Deletes the SageMaker model mxnet_predictor.delete_model() The example above will eventually delete both the SageMaker endpoint and endpoint configuration through `delete_endpoint()`. If you want to keep your SageMaker endpoint configuration, use the value False for the `delete_endpoint_config` parameter, as shown below. @@ -232,7 +232,7 @@ For more `information Date: Thu, 21 Feb 2019 10:46:27 -0800 Subject: [PATCH 10/13] Continue deleting models with failed delete requests. --- src/sagemaker/predictor.py | 12 ++++++++---- tests/unit/test_predictor.py | 2 +- 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/src/sagemaker/predictor.py b/src/sagemaker/predictor.py index b86e047023..ff4ac3899b 100644 --- a/src/sagemaker/predictor.py +++ b/src/sagemaker/predictor.py @@ -132,11 +132,15 @@ def delete_model(self): """Deletes the Amazon SageMaker models backing this predictor. """ - try: - for model_name in self._model_names: + request_failed = False + for model_name in self._model_names: + try: self.sagemaker_session.delete_model(model_name) - except Exception: - raise Exception('One or more models cannot be deleted, the deletion is incomplete.') + except Exception: # pylint: disable=broad-except + request_failed = True + + if request_failed: + raise Exception('One or more models cannot be deleted, please retry.') def _get_endpoint_config_name(self): endpoint_desc = self.sagemaker_session.sagemaker_client.describe_endpoint(EndpointName=self.endpoint) diff --git a/tests/unit/test_predictor.py b/tests/unit/test_predictor.py index d2bc1fc71d..ea71bb5715 100644 --- a/tests/unit/test_predictor.py +++ b/tests/unit/test_predictor.py @@ -504,7 +504,7 @@ def test_delete_model(): def test_delete_model_fail(): sagemaker_session = empty_sagemaker_session() sagemaker_session.sagemaker_client.delete_model = Mock(side_effect='Could not find model.') - expected_error_message = 'One or more models cannot be deleted, the deletion is incomplete.' + expected_error_message = 'One or more models cannot be deleted, please retry.' predictor = RealTimePredictor(ENDPOINT, sagemaker_session=sagemaker_session) From 204b197cd0d553cacd15b98f771fad1179d04421 Mon Sep 17 00:00:00 2001 From: Dan Date: Thu, 21 Feb 2019 11:06:20 -0800 Subject: [PATCH 11/13] add sphinx-build in tox (#653) --- tox.ini | 27 ++++++++++++++++++++++++++- 1 file changed, 26 insertions(+), 1 deletion(-) diff --git a/tox.ini b/tox.ini index 6402496c88..4e081b28bf 100644 --- a/tox.ini +++ b/tox.ini @@ -4,7 +4,7 @@ # and then run "tox" from this directory. [tox] -envlist = py27,py35,flake8,pylint +envlist = py27,py35,flake8,pylint,sphinx skip_missing_interpreters = False [travis] @@ -72,3 +72,28 @@ deps = pylint==2.1.1 commands = python -m pylint --rcfile=.pylintrc -j 0 src/sagemaker + +[testenv:sphinx] +basepython = python3 +changedir = doc +# Based on: https://github.com/rtfd/readthedocs.org/blob/8f0c78dde5edcc85acf90462a8518735a25482d3/readthedocs/doc_builder/python_environments.py#L263 +install_command = python -m pip install --upgrade -I {packages} +# Based on: https://github.com/rtfd/readthedocs.org/blob/8f0c78dde5edcc85acf90462a8518735a25482d3/readthedocs/doc_builder/python_environments.py#L280 +deps = + Pygments==2.2.0 + setuptools<40 + docutils==0.13.1 + mock==1.0.1 + pillow==2.6.1 + alabaster>=0.7,<0.8,!=0.7.5 + commonmark==0.5.4 + recommonmark==0.4.0 + sphinx<1.8 + sphinx-rtd-theme<0.5 + readthedocs-sphinx-ext<0.6 +# pip install requirements.txt is separate as RTD does it in separate steps +# having the requirements.txt installed in deps above results in Double Requirement exception +# https://github.com/pypa/pip/issues/988 +commands = + pip install --exists-action=w -r requirements.txt + sphinx-build -T -W -b html -d _build/doctrees-readthedocs -D language=en . _build/html From be7e86c7c3513d1daf794b6142f54a955e9768c9 Mon Sep 17 00:00:00 2001 From: Dan Date: Thu, 21 Feb 2019 11:15:13 -0800 Subject: [PATCH 12/13] add twine check in tox (#651) --- tox.ini | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/tox.ini b/tox.ini index 4e081b28bf..cb5d226abd 100644 --- a/tox.ini +++ b/tox.ini @@ -4,7 +4,8 @@ # and then run "tox" from this directory. [tox] -envlist = py27,py35,flake8,pylint,sphinx +envlist = py27,py35,flake8,pylint,twine,sphinx + skip_missing_interpreters = False [travis] @@ -73,6 +74,17 @@ deps = commands = python -m pylint --rcfile=.pylintrc -j 0 src/sagemaker +[testenv:twine] +basepython = python +# twine check was added starting in 1.12.0 +# https://github.com/pypa/twine/blob/master/docs/changelog.rst +deps = + twine>=1.12.0 +# https://packaging.python.org/guides/making-a-pypi-friendly-readme/#validating-restructuredtext-markup +commands = + python setup.py sdist + twine check dist/*.tar.gz + [testenv:sphinx] basepython = python3 changedir = doc From af7628913fd91d846543ee1e0581a5a0c4080d83 Mon Sep 17 00:00:00 2001 From: Chuyang Deng Date: Thu, 21 Feb 2019 11:57:52 -0800 Subject: [PATCH 13/13] Print failed model delete names in error message. --- src/sagemaker/predictor.py | 5 ++++- tests/unit/test_predictor.py | 4 ++-- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/src/sagemaker/predictor.py b/src/sagemaker/predictor.py index ff4ac3899b..5da69dfb2f 100644 --- a/src/sagemaker/predictor.py +++ b/src/sagemaker/predictor.py @@ -133,14 +133,17 @@ def delete_model(self): """ request_failed = False + failed_models = [] for model_name in self._model_names: try: self.sagemaker_session.delete_model(model_name) except Exception: # pylint: disable=broad-except request_failed = True + failed_models.append(model_name) if request_failed: - raise Exception('One or more models cannot be deleted, please retry.') + raise Exception('One or more models cannot be deleted, please retry. \n' + 'Failed models: {}'.format(', '.join(failed_models))) def _get_endpoint_config_name(self): endpoint_desc = self.sagemaker_session.sagemaker_client.describe_endpoint(EndpointName=self.endpoint) diff --git a/tests/unit/test_predictor.py b/tests/unit/test_predictor.py index ea71bb5715..2b1434c580 100644 --- a/tests/unit/test_predictor.py +++ b/tests/unit/test_predictor.py @@ -503,11 +503,11 @@ def test_delete_model(): def test_delete_model_fail(): sagemaker_session = empty_sagemaker_session() - sagemaker_session.sagemaker_client.delete_model = Mock(side_effect='Could not find model.') + sagemaker_session.sagemaker_client.delete_model = Mock(side_effect=Exception('Could not find model.')) expected_error_message = 'One or more models cannot be deleted, please retry.' predictor = RealTimePredictor(ENDPOINT, sagemaker_session=sagemaker_session) with pytest.raises(Exception) as exception: predictor.delete_model() - assert str(exception.val) == expected_error_message + assert expected_error_message in str(exception.val)