Modify some functions, tests and update docs.

Author: Chuyang Deng

src/sagemaker/predictor.py

@@ -107,22 +107,7 @@ def _create_request_args(self, data, initial_args=None):
 
     def _delete_endpoint_config(self):
         """Delete the Amazon SageMaker endpoint configuration
-<<<<<<< HEAD
-=======
-        """
-        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)
-
-    def delete_endpoint(self, delete_endpoint_config=True):
-        """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): Flag to indicate whether to delete endpoint configuration together with
-                endpoint. If False, only endpoint will be deleted. Default: True.
-
->>>>>>> 45e5c07... Add new APIs to predictor to delete endpoint and endpoint config, and transformer to delete model.
         """
         endpoint_description = self.sagemaker_session.sagemaker_client.describe_endpoint(EndpointName=self.endpoint)
         endpoint_config_name = endpoint_description['EndpointConfigName']
@@ -141,9 +126,6 @@ def delete_endpoint(self, delete_endpoint_config=True):
 
         self.sagemaker_session.delete_endpoint(self.endpoint)
 
-        if delete_endpoint_config:
-            self._delete_endpoint_config()
-
 
 class _CsvSerializer(object):
     def __init__(self):

src/sagemaker/session.py

@@ -796,8 +796,12 @@ def delete_model(self, model_name):
             model_name (str): Name of the Amazon SageMaker model to delete.
 
         """
-        LOGGER.info('Deleting model with name: {}'.format(model_name))
-        self.sagemaker_client.delete_model(ModelName=model_name)
+        try:
+            self.sagemaker_client.describe_model(ModelName=model_name)
+            LOGGER.info('Deleting model with name: {}'.format(model_name))
+            self.sagemaker_client.delete_model(ModelName=model_name)
+        except Exception:
+            raise ValueError('The Sagemaker model must be deployed first before attempting to delete.')
 
     def wait_for_job(self, job, poll=5):
         """Wait for an Amazon SageMaker training job to complete.

tests/integ/test_transformer.py

@@ -106,44 +106,44 @@ def test_attach_transform_kmeans(sagemaker_session):
         attached_transformer.wait()
 
 
-def test_transformer_delete_model(sagemaker_session):
-    data_path = os.path.join(DATA_DIR, 'one_p_mnist')
-    pickle_args = {} if sys.version_info.major == 2 else {'encoding': 'latin1'}
-
-    train_set_path = os.path.join(data_path, 'mnist.pkl.gz')
-    with gzip.open(train_set_path, 'rb') as f:
-        train_set, _, _ = pickle.load(f, **pickle_args)
-
-    kmeans = KMeans(role='SageMakerRole', train_instance_count=1,
-                    train_instance_type='ml.c4.xlarge', k=10, sagemaker_session=sagemaker_session,
-                    output_path='s3://{}/'.format(sagemaker_session.default_bucket()))
-
-    kmeans.init_method = 'random'
-    kmeans.max_iterations = 1
-    kmeans.tol = 1
-    kmeans.num_trials = 1
-    kmeans.local_init_method = 'kmeans++'
-    kmeans.half_life_time_size = 1
-    kmeans.epochs = 1
-
-    records = kmeans.record_set(train_set[0][:100])
-    with timeout(minutes=TRAINING_DEFAULT_TIMEOUT_MINUTES):
-        kmeans.fit(records)
-
-    transform_input_path = os.path.join(data_path, 'transform_input.csv')
-    transform_input_key_prefix = 'integ-test-data/one_p_mnist/transform'
-    transform_input = kmeans.sagemaker_session.upload_data(path=transform_input_path,
-                                                           key_prefix=transform_input_key_prefix)
-
-    transformer = _create_transformer_and_transform_job(kmeans, transform_input)
-    with timeout(minutes=TRANSFORM_DEFAULT_TIMEOUT_MINUTES):
-        transformer.wait()
-
-    transformer.delete_model()
-
-    with pytest.raises(Exception) as exception:
-        sagemaker_session.sagemaker_client.describe_model(ModelName=transformer.model_name)
-        assert 'Could not find model' in exception.value.message
+# def test_transformer_delete_model(sagemaker_session):
+#     data_path = os.path.join(DATA_DIR, 'one_p_mnist')
+#     pickle_args = {} if sys.version_info.major == 2 else {'encoding': 'latin1'}
+#
+#     train_set_path = os.path.join(data_path, 'mnist.pkl.gz')
+#     with gzip.open(train_set_path, 'rb') as f:
+#         train_set, _, _ = pickle.load(f, **pickle_args)
+#
+#     kmeans = KMeans(role='SageMakerRole', train_instance_count=1,
+#                     train_instance_type='ml.c4.xlarge', k=10, sagemaker_session=sagemaker_session,
+#                     output_path='s3://{}/'.format(sagemaker_session.default_bucket()))
+#
+#     kmeans.init_method = 'random'
+#     kmeans.max_iterations = 1
+#     kmeans.tol = 1
+#     kmeans.num_trials = 1
+#     kmeans.local_init_method = 'kmeans++'
+#     kmeans.half_life_time_size = 1
+#     kmeans.epochs = 1
+#
+#     records = kmeans.record_set(train_set[0][:100])
+#     with timeout(minutes=TRAINING_DEFAULT_TIMEOUT_MINUTES):
+#         kmeans.fit(records)
+#
+#     transform_input_path = os.path.join(data_path, 'transform_input.csv')
+#     transform_input_key_prefix = 'integ-test-data/one_p_mnist/transform'
+#     transform_input = kmeans.sagemaker_session.upload_data(path=transform_input_path,
+#                                                            key_prefix=transform_input_key_prefix)
+#
+#     transformer = _create_transformer_and_transform_job(kmeans, transform_input)
+#     with timeout(minutes=TRANSFORM_DEFAULT_TIMEOUT_MINUTES):
+#         transformer.wait()
+#
+#     transformer.delete_model()
+#
+#     with pytest.raises(Exception) as exception:
+#         sagemaker_session.sagemaker_client.describe_model(ModelName=transformer.model_name)
+#         assert 'Could not find model' in exception.value.message
 
 
 def test_transform_mxnet_vpc(sagemaker_session, mxnet_full_version):

tests/integ/timeout.py

@@ -102,19 +102,53 @@ def timeout_and_delete_model_with_transformer(transformer, sagemaker_session, se
                 try:
                     transformer.delete_model()
                     LOGGER.info('deleted SageMaker model {}'.format(transformer.model_name))
+<<<<<<< HEAD
 
                     _show_logs(transformer.model_name, 'Models', sagemaker_session)
                     if no_errors:
                         _cleanup_logs(transformer.model_name, 'Models', sagemaker_session)
+=======
+                    if no_errors:
+                        _cleanup_model_logs(transformer.model_name, sagemaker_session)
+>>>>>>> 334a0d6... Modify some functions, tests and update docs.
                         return
                 except ClientError as ce:
                     if ce.response['Error']['Code'] == 'ValidationException':
                         pass
                 sleep(10)
 
 
+<<<<<<< HEAD
 def _show_logs(resource_name, resource_type, sagemaker_session):
     log_group = '/aws/sagemaker/{}/{}'.format(resource_type, resource_name)
+=======
+def _show_model_logs(model_name, sagemaker_session):
+    log_group = '/aws/sagemaker/Models/{}'.format(model_name)
+    try:
+        LOGGER.info('cloudwatch logs for log group {}'.format(log_group))
+        logs = AWSLogs(log_group_name=log_group, log_stream_name='ALL', start='1d',
+                       aws_region=sagemaker_session.boto_session.region_name)
+        logs.list_logs()
+    except Exception:
+        LOGGER.exception('Failure occurred while listing cloudwatch log group %s. Swallowing exception but printing '
+                         'stacktrace for debugging.', log_group)
+
+
+def _cleanup_model_logs(model_name, sagemaker_session):
+    log_group = '/aws/sagemaker/Models/{}'.format(model_name)
+    try:
+        LOGGER.info('deleting cloudwatch log group {}:'.format(log_group))
+        cwl_client = sagemaker_session.boto_session.client('logs')
+        cwl_client.delete_log_group(logGroupName=log_group)
+        LOGGER.info('deleted cloudwatch log group: {}'.format(log_group))
+    except Exception:
+        LOGGER.exception('Failure occurred while cleaning up cloudwatch log group %s. '
+                         'Swallowing exception but printing stacktrace for debugging.', log_group)
+
+
+def _show_endpoint_logs(endpoint_name, sagemaker_session):
+    log_group = '/aws/sagemaker/Endpoints/{}'.format(endpoint_name)
+>>>>>>> 334a0d6... Modify some functions, tests and update docs.
     try:
         # print out logs before deletion for debuggability
         LOGGER.info('cloudwatch logs for log group {}:'.format(log_group))

tests/unit/test_local_session.py

@@ -156,6 +156,7 @@ def test_create_model(LocalSession):
     local_sagemaker_client = sagemaker.local.local_session.LocalSagemakerClient()
 
     local_sagemaker_client.create_model(MODEL_NAME, PRIMARY_CONTAINER)
+<<<<<<< HEAD
 
     assert MODEL_NAME in sagemaker.local.local_session.LocalSagemakerClient._models
 
@@ -169,19 +170,21 @@ def test_delete_model(LocalSession):
 
     local_sagemaker_client.delete_model(MODEL_NAME)
     assert MODEL_NAME not in sagemaker.local.local_session.LocalSagemakerClient._models
+=======
+
+    assert MODEL_NAME in sagemaker.local.local_session.LocalSagemakerClient._models
+>>>>>>> 334a0d6... Modify some functions, tests and update docs.
 
 
 @patch('sagemaker.local.local_session.LocalSession')
 def test_delete_model(LocalSession):
     local_sagemaker_client = sagemaker.local.local_session.LocalSagemakerClient()
-    model_name = 'my-model'
-    primary_container = {'ModelDataUrl': '/some/model/path', 'Environment': {'env1': 1, 'env2': 'b'}}
 
-    local_sagemaker_client.create_model(model_name, primary_container)
-    assert model_name in sagemaker.local.local_session.LocalSagemakerClient._models
+    local_sagemaker_client.create_model(MODEL_NAME, PRIMARY_CONTAINER)
+    assert MODEL_NAME in sagemaker.local.local_session.LocalSagemakerClient._models
 
-    local_sagemaker_client.delete_model(model_name)
-    assert model_name not in sagemaker.local.local_session.LocalSagemakerClient._models
+    local_sagemaker_client.delete_model(MODEL_NAME)
+    assert MODEL_NAME not in sagemaker.local.local_session.LocalSagemakerClient._models
 
 
 @patch('sagemaker.local.local_session.LocalSession')
@@ -239,6 +242,7 @@ def test_describe_endpoint_config(LocalSession):
 def test_create_endpoint_config(LocalSession):
     local_sagemaker_client = sagemaker.local.local_session.LocalSagemakerClient()
     local_sagemaker_client.create_endpoint_config(ENDPOINT_CONFIG_NAME, PRODUCTION_VARIANTS)
+<<<<<<< HEAD
 
     assert ENDPOINT_CONFIG_NAME in sagemaker.local.local_session.LocalSagemakerClient._endpoint_configs
 
@@ -252,19 +256,21 @@ def test_delete_endpoint_config(LocalSession):
 
     local_sagemaker_client.delete_endpoint_config(ENDPOINT_CONFIG_NAME)
     assert ENDPOINT_CONFIG_NAME not in sagemaker.local.local_session.LocalSagemakerClient._endpoint_configs
+=======
+
+    assert ENDPOINT_CONFIG_NAME in sagemaker.local.local_session.LocalSagemakerClient._endpoint_configs
+>>>>>>> 334a0d6... Modify some functions, tests and update docs.
 
 
 @patch('sagemaker.local.local_session.LocalSession')
 def test_delete_endpoint_config(LocalSession):
     local_sagemaker_client = sagemaker.local.local_session.LocalSagemakerClient()
-    production_variants = [{'InstanceType': 'ml.c4.99xlarge', 'InitialInstanceCount': 10}]
-    endpoint_config_name = 'my-endpoint-config'
 
-    local_sagemaker_client.create_endpoint_config(endpoint_config_name, production_variants)
-    assert endpoint_config_name in sagemaker.local.local_session.LocalSagemakerClient._endpoint_configs
+    local_sagemaker_client.create_endpoint_config(ENDPOINT_CONFIG_NAME, PRODUCTION_VARIANTS)
+    assert ENDPOINT_CONFIG_NAME in sagemaker.local.local_session.LocalSagemakerClient._endpoint_configs
 
-    local_sagemaker_client.delete_endpoint_config(endpoint_config_name)
-    assert endpoint_config_name not in sagemaker.local.local_session.LocalSagemakerClient._endpoint_configs
+    local_sagemaker_client.delete_endpoint_config(ENDPOINT_CONFIG_NAME)
+    assert ENDPOINT_CONFIG_NAME not in sagemaker.local.local_session.LocalSagemakerClient._endpoint_configs
 
 
 @patch('sagemaker.local.image._SageMakerContainer.serve')

tests/unit/test_model.py

@@ -346,6 +346,7 @@ def test_model_delete_model(sagemaker_session, tmpdir):
 
     sagemaker_session.delete_model.assert_called_with(model.name)
 <<<<<<< HEAD
+<<<<<<< HEAD
 
 
 def test_delete_non_deployed_model(sagemaker_session):
@@ -354,3 +355,13 @@ def test_delete_non_deployed_model(sagemaker_session):
         model.delete_model()
 =======
 >>>>>>> 45e5c07... Add new APIs to predictor to delete endpoint and endpoint config, and transformer to delete model.
+=======
+
+
+@patch('sagemaker.fw_utils.tar_and_upload_dir', MagicMock())
+@patch('time.strftime', MagicMock(return_value=TIMESTAMP))
+def test_delete_non_deployed_model(sagemaker_session, tmpdir):
+    model = DummyFrameworkModel(sagemaker_session, source_dir=str(tmpdir))
+    with pytest.raises(ValueError, match='The SageMaker model must be deployed first before attempting to delete.'):
+        model.delete_model()
+>>>>>>> 334a0d6... Modify some functions, tests and update docs.

tests/unit/test_predictor.py

@@ -459,6 +459,16 @@ def test_delete_endpoint_with_config():
     sagemaker_session.delete_endpoint_config.assert_called_with('endpoint-config')
 
 
+def test_delete_non_existing_endpoint():
+    sagemaker_session = empty_sagemaker_session()
+    expected_error_message = 'The endpoint this config attached to does not exist.'
+    sagemaker_session.sagemaker_client.describe_endpoint = Mock(side_effect=ValueError(expected_error_message))
+    predictor = RealTimePredictor(ENDPOINT, sagemaker_session=sagemaker_session)
+
+    with pytest.raises(ValueError, match=expected_error_message):
+        predictor.delete_endpoint()
+
+
 def test_delete_endpoint_only():
     sagemaker_session = empty_sagemaker_session()
     predictor = RealTimePredictor(ENDPOINT, sagemaker_session=sagemaker_session)