Fix bug in local mode (#429)

* Fix bug in localmode
Set correct default values for  additional_env_vars and additional_volumes and add unit test.

* Modify changelog

Author: icywang86rui

CHANGELOG.rst

@@ -7,9 +7,9 @@ CHANGELOG
 
 * feature: Local Mode: Add support for Batch Inference
 * feature: Add timestamp to secondary status in training job output
+* bug-fix: Local Mode: Set correct default values for additional_volumes and additional_env_vars
 * enhancement: Local Mode: support nvidia-docker2 natively
 
-
 1.11.2
 ======
 

src/sagemaker/local/image.py

@@ -341,8 +341,8 @@ def _generate_compose_file(self, command, additional_volumes=None, additional_en
 
         """
         boto_session = self.sagemaker_session.boto_session
-        additional_env_vars = additional_env_vars or []
-        additional_volumes = additional_volumes or {}
+        additional_volumes = additional_volumes or []
+        additional_env_vars = additional_env_vars or {}
         environment = []
         optml_dirs = set()
 

src/sagemaker/utils.py

@@ -160,7 +160,7 @@ def secondary_training_status_changed(current_job_description, prev_job_descript
 
 
 def secondary_training_status_message(job_description, prev_description):
-    """Returns a string contains start time and the secondary training job status message.
+    """Returns a string contains last modified time and the secondary training job status message.
 
     Args:
         job_description: Returned response from DescribeTrainingJob call
@@ -181,8 +181,12 @@ def secondary_training_status_message(job_description, prev_description):
         if prev_description_secondary_transitions is not None else 0
     current_transitions = job_description['SecondaryStatusTransitions']
 
-    transitions_to_print = current_transitions[-1:] if len(current_transitions) == prev_transitions_num else \
-        current_transitions[prev_transitions_num - len(current_transitions):]
+    if len(current_transitions) == prev_transitions_num:
+        # Secondary status is not changed but the message changed.
+        transitions_to_print = current_transitions[-1:]
+    else:
+        # Secondary status is changed we need to print all the entries.
+        transitions_to_print = current_transitions[prev_transitions_num - len(current_transitions):]
 
     status_strs = []
     for transition in transitions_to_print:

tests/unit/test_image.py

@@ -413,6 +413,28 @@ def test_serve_local_code(up, copy, copytree, tmpdir, sagemaker_session):
                 assert '%s:/opt/ml/code' % '/tmp/code' in volumes
 
 
+@patch('sagemaker.local.image._HostingContainer.run')
+@patch('shutil.copy')
+@patch('shutil.copytree')
+def test_serve_local_code_no_env(up, copy, copytree, tmpdir, sagemaker_session):
+
+    with patch('sagemaker.local.image._SageMakerContainer._create_tmp_folder',
+               return_value=str(tmpdir.mkdir('container-root'))):
+
+        image = 'my-image'
+        sagemaker_container = _SageMakerContainer('local', 1, image, sagemaker_session=sagemaker_session)
+        sagemaker_container.serve('/some/model/path', {})
+        docker_compose_file = os.path.join(sagemaker_container.container_root,
+                                           'docker-compose.yaml')
+
+        with open(docker_compose_file, 'r') as f:
+            config = yaml.load(f)
+
+            for h in sagemaker_container.hosts:
+                assert config['services'][h]['image'] == image
+                assert config['services'][h]['command'] == 'serve'
+
+
 @patch('sagemaker.utils.download_file')
 @patch('tarfile.is_tarfile')
 @patch('tarfile.open', MagicMock())