don't allow for absolute path entry_point with S3 source_dir

Author: laurenyu

src/sagemaker/chainer/estimator.py

@@ -216,7 +216,7 @@ def create_model(
         return ChainerModel(
             self.model_data,
             role or self.role,
-            entry_point or self.uploaded_code.script_name,
+            entry_point or self._model_entry_point(),
             source_dir=(source_dir or self._model_source_dir()),
             enable_cloudwatch_metrics=self.enable_cloudwatch_metrics,
             container_log_level=self.container_log_level,

src/sagemaker/estimator.py

@@ -1755,17 +1755,25 @@ def _stage_user_code_in_s3(self):
         )
 
     def _model_source_dir(self):
-        """Get the appropriate value to pass as source_dir to model constructor
-        on deploying
+        """Get the appropriate value to pass as ``source_dir`` to a model constructor.
 
         Returns:
-            str: Either a local or an S3 path pointing to the source_dir to be
-            used for code by the model to be deployed
+            str: Either a local or an S3 path pointing to the ``source_dir`` to be
+                used for code by the model to be deployed
         """
         return (
             self.source_dir if self.sagemaker_session.local_mode else self.uploaded_code.s3_prefix
         )
 
+    def _model_entry_point(self):
+        """Get the appropriate value to pass as ``entry_point`` to a model constructor.
+
+        Returns:
+            str: The path to the entry point script. This can be either an absolute path or
+                a path relative to ``self._model_source_dir()``.
+        """
+        return self.uploaded_code.script_name if self._model_source_dir() else self.entry_point
+
     def hyperparameters(self):
         """Return the hyperparameters as a dictionary to use for training.
 

src/sagemaker/mxnet/estimator.py

@@ -217,10 +217,10 @@ def create_model(
         if "name" not in kwargs:
             kwargs["name"] = self._current_job_name
 
-        model = MXNetModel(
+        return MXNetModel(
             self.model_data,
             role or self.role,
-            entry_point,
+            entry_point or self._model_entry_point(),
             source_dir=(source_dir or self._model_source_dir()),
             enable_cloudwatch_metrics=self.enable_cloudwatch_metrics,
             container_log_level=self.container_log_level,
@@ -234,13 +234,6 @@ def create_model(
             **kwargs
         )
 
-        if entry_point is None:
-            model.entry_point = (
-                self.entry_point if model._is_mms_version() else self.uploaded_code.script_name
-            )
-
-        return model
-
     @classmethod
     def _prepare_init_params_from_job_description(cls, job_details, model_channel_name=None):
         """Convert the job description to init params that can be handled by the

src/sagemaker/pytorch/estimator.py

@@ -176,7 +176,7 @@ def create_model(
         return PyTorchModel(
             self.model_data,
             role or self.role,
-            entry_point or self.uploaded_code.script_name,
+            entry_point or self._model_entry_point(),
             source_dir=(source_dir or self._model_source_dir()),
             enable_cloudwatch_metrics=self.enable_cloudwatch_metrics,
             container_log_level=self.container_log_level,

src/sagemaker/rl/estimator.py

@@ -229,7 +229,7 @@ def create_model(
         if not entry_point and (source_dir or dependencies):
             raise AttributeError("Please provide an `entry_point`.")
 
-        entry_point = entry_point or self.uploaded_code.script_name
+        entry_point = entry_point or self._model_entry_point()
         source_dir = source_dir or self._model_source_dir()
         dependencies = dependencies or self.dependencies
 

src/sagemaker/sklearn/estimator.py

@@ -194,7 +194,7 @@ def create_model(
         return SKLearnModel(
             self.model_data,
             role,
-            entry_point or self.uploaded_code.script_name,
+            entry_point or self._model_entry_point(),
             source_dir=(source_dir or self._model_source_dir()),
             enable_cloudwatch_metrics=self.enable_cloudwatch_metrics,
             container_log_level=self.container_log_level,

src/sagemaker/tensorflow/estimator.py

@@ -660,7 +660,7 @@ def _create_default_model(
         return TensorFlowModel(
             self.model_data,
             role,
-            entry_point or self.uploaded_code.script_name,
+            entry_point or self._model_entry_point(),
             source_dir=source_dir or self._model_source_dir(),
             enable_cloudwatch_metrics=self.enable_cloudwatch_metrics,
             env={"SAGEMAKER_REQUIREMENTS": self.requirements_file},

src/sagemaker/xgboost/estimator.py

@@ -174,7 +174,7 @@ def create_model(
         return XGBoostModel(
             self.model_data,
             role,
-            entry_point or self.uploaded_code.script_name,
+            entry_point or self._model_entry_point(),
             framework_version=self.framework_version,
             source_dir=(source_dir or self._model_source_dir()),
             enable_cloudwatch_metrics=self.enable_cloudwatch_metrics,

tests/integ/test_mxnet_train.py

@@ -39,7 +39,7 @@ def mxnet_training_job(sagemaker_session, mxnet_full_version, cpu_instance_type)
         )
 
         mx = MXNet(
-            entry_point=os.path.join("mxnet_mnist", "mnist.py"),
+            entry_point="mxnet_mnist/mnist.py",
             source_dir=s3_source,
             role="SageMakerRole",
             framework_version=mxnet_full_version,

tests/unit/test_mxnet.py

@@ -180,15 +180,15 @@ def test_create_model(sagemaker_session, mxnet_version):
     container_log_level = '"logging.INFO"'
     source_dir = "s3://mybucket/source"
     mx = MXNet(
-        entry_point=SCRIPT_PATH,
+        entry_point=SCRIPT_NAME,
+        source_dir=source_dir,
         role=ROLE,
         sagemaker_session=sagemaker_session,
         train_instance_count=INSTANCE_COUNT,
         train_instance_type=INSTANCE_TYPE,
         framework_version=mxnet_version,
         container_log_level=container_log_level,
         base_job_name="job",
-        source_dir=source_dir,
     )
 
     job_name = "new_name"
@@ -198,6 +198,7 @@ def test_create_model(sagemaker_session, mxnet_version):
     assert model.sagemaker_session == sagemaker_session
     assert model.framework_version == mxnet_version
     assert model.py_version == mx.py_version
+    assert model.entry_point == SCRIPT_NAME
     assert model.role == ROLE
     assert model.name == job_name
     assert model.container_log_level == container_log_level
@@ -206,55 +207,19 @@ def test_create_model(sagemaker_session, mxnet_version):
     assert model.vpc_config is None
 
 
-@patch("sagemaker.utils.create_tar_file", MagicMock())
-def test_create_model_default_entry_with_mms(
-    sagemaker_session, mxnet_version, skip_if_not_mms_version
-):
-    mx = MXNet(
-        entry_point=SCRIPT_PATH,
-        role=ROLE,
-        sagemaker_session=sagemaker_session,
-        train_instance_count=INSTANCE_COUNT,
-        train_instance_type=INSTANCE_TYPE,
-        framework_version=mxnet_version,
-    )
-
-    mx.fit()
-    model = mx.create_model()
-
-    assert model.entry_point == SCRIPT_PATH
-
-
-@patch("sagemaker.utils.create_tar_file", MagicMock())
-def test_create_model_default_entry_not_mms(sagemaker_session, mxnet_version, skip_if_mms_version):
-    mx = MXNet(
-        entry_point=SCRIPT_PATH,
-        role=ROLE,
-        sagemaker_session=sagemaker_session,
-        train_instance_count=INSTANCE_COUNT,
-        train_instance_type=INSTANCE_TYPE,
-        framework_version=mxnet_version,
-    )
-
-    mx.fit()
-    model = mx.create_model()
-
-    assert model.entry_point == SCRIPT_NAME
-
-
 def test_create_model_with_optional_params(sagemaker_session):
     container_log_level = '"logging.INFO"'
     source_dir = "s3://mybucket/source"
     enable_cloudwatch_metrics = "true"
     mx = MXNet(
-        entry_point=SCRIPT_PATH,
+        entry_point=SCRIPT_NAME,
+        source_dir=source_dir,
         role=ROLE,
         sagemaker_session=sagemaker_session,
         train_instance_count=INSTANCE_COUNT,
         train_instance_type=INSTANCE_TYPE,
         container_log_level=container_log_level,
         base_job_name="job",
-        source_dir=source_dir,
         enable_cloudwatch_metrics=enable_cloudwatch_metrics,
     )
 
@@ -286,15 +251,15 @@ def test_create_model_with_custom_image(sagemaker_session):
     source_dir = "s3://mybucket/source"
     custom_image = "mxnet:2.0"
     mx = MXNet(
-        entry_point=SCRIPT_PATH,
+        entry_point=SCRIPT_NAME,
+        source_dir=source_dir,
         role=ROLE,
         sagemaker_session=sagemaker_session,
         train_instance_count=INSTANCE_COUNT,
         train_instance_type=INSTANCE_TYPE,
         image_name=custom_image,
         container_log_level=container_log_level,
         base_job_name="job",
-        source_dir=source_dir,
     )
 
     job_name = "new_name"
@@ -303,7 +268,7 @@ def test_create_model_with_custom_image(sagemaker_session):
 
     assert model.sagemaker_session == sagemaker_session
     assert model.image == custom_image
-    assert model.entry_point == SCRIPT_PATH
+    assert model.entry_point == SCRIPT_NAME
     assert model.role == ROLE
     assert model.name == job_name
     assert model.container_log_level == container_log_level
@@ -823,7 +788,6 @@ def test_create_model_with_custom_hosting_image(sagemaker_session):
         image_name=custom_image,
         container_log_level=container_log_level,
         base_job_name="job",
-        source_dir=source_dir,
     )
 
     mx.fit(inputs="s3://mybucket/train", job_name="new_name")