Merge branch 'master' into fix-spark-processor

Author: jerrypeng7773

doc/frameworks/xgboost/using_xgboost.rst

@@ -465,6 +465,7 @@ For information about the SageMaker Python SDK XGBoost classes, see the followin
 * :class:`sagemaker.xgboost.estimator.XGBoost`
 * :class:`sagemaker.xgboost.model.XGBoostModel`
 * :class:`sagemaker.xgboost.model.XGBoostPredictor`
+* :class:`sagemaker.xgboost.processing.XGBoostProcessor`
 
 ***********************************
 SageMaker XGBoost Docker Containers

doc/frameworks/xgboost/xgboost.rst

@@ -16,3 +16,8 @@ The Amazon SageMaker XGBoost open source framework algorithm.
     :members:
     :undoc-members:
     :show-inheritance:
+
+.. autoclass:: sagemaker.xgboost.processing.XGBoostProcessor
+    :members:
+    :undoc-members:
+    :show-inheritance:

src/sagemaker/model.py

@@ -341,7 +341,9 @@ def register(
                 "MACHINE_LEARNING" (default: None).
 
         Returns:
-            A `sagemaker.model.ModelPackage` instance.
+            A `sagemaker.model.ModelPackage` instance or pipeline step arguments
+            in case the Model instance is built with
+            :class:`~sagemaker.workflow.pipeline_context.PipelineSession`
         """
         if self.model_data is None:
             raise ValueError("SageMaker Model Package cannot be created without model data.")
@@ -398,15 +400,22 @@ def create(
                 attach to an endpoint for model loading and inference, for
                 example, 'ml.eia1.medium'. If not specified, no Elastic
                 Inference accelerator will be attached to the endpoint (default: None).
-            serverless_inference_config (sagemaker.serverless.ServerlessInferenceConfig):
+            serverless_inference_config (ServerlessInferenceConfig):
                 Specifies configuration related to serverless endpoint. Instance type is
                 not provided in serverless inference. So this is used to find image URIs
                 (default: None).
             tags (List[Dict[str, str]]): The list of tags to add to
-                the model (default: None). Example: >>> tags = [{'Key': 'tagname', 'Value':
-                'tagvalue'}] For more information about tags, see
-                https://boto3.amazonaws.com/v1/documentation
-                /api/latest/reference/services/sagemaker.html#SageMaker.Client.add_tags
+                the model (default: None). Example::
+
+                    tags = [{'Key': 'tagname', 'Value':'tagvalue'}]
+
+                For more information about tags, see
+                `boto3 documentation <https://boto3.amazonaws.com/v1/documentation/\
+api/latest/reference/services/sagemaker.html#SageMaker.Client.add_tags>`_
+
+        Returns:
+            None or pipeline step arguments in case the Model instance is built with
+            :class:`~sagemaker.workflow.pipeline_context.PipelineSession`
         """
         # TODO: we should replace _create_sagemaker_model() with create()
         self._create_sagemaker_model(

src/sagemaker/session.py

@@ -412,29 +412,47 @@ def _create_s3_bucket_if_it_does_not_exist(self, bucket_name, region):
         bucket = s3.Bucket(name=bucket_name)
         if bucket.creation_date is None:
             try:
-                if region == "us-east-1":
-                    # 'us-east-1' cannot be specified because it is the default region:
-                    # https://github.com/boto/boto3/issues/125
-                    s3.create_bucket(Bucket=bucket_name)
-                else:
-                    s3.create_bucket(
-                        Bucket=bucket_name, CreateBucketConfiguration={"LocationConstraint": region}
-                    )
-
-                LOGGER.info("Created S3 bucket: %s", bucket_name)
+                # trying head bucket call
+                s3.meta.client.head_bucket(Bucket=bucket.name)
             except ClientError as e:
+                # bucket does not exist or forbidden to access
                 error_code = e.response["Error"]["Code"]
                 message = e.response["Error"]["Message"]
 
-                if error_code == "BucketAlreadyOwnedByYou":
-                    pass
-                elif (
-                    error_code == "OperationAborted"
-                    and "conflicting conditional operation" in message
-                ):
-                    # If this bucket is already being concurrently created, we don't need to create
-                    # it again.
-                    pass
+                if error_code == "404" and message == "Not Found":
+                    # bucket does not exist, create one
+                    try:
+                        if region == "us-east-1":
+                            # 'us-east-1' cannot be specified because it is the default region:
+                            # https://github.com/boto/boto3/issues/125
+                            s3.create_bucket(Bucket=bucket_name)
+                        else:
+                            s3.create_bucket(
+                                Bucket=bucket_name,
+                                CreateBucketConfiguration={"LocationConstraint": region},
+                            )
+
+                        LOGGER.info("Created S3 bucket: %s", bucket_name)
+                    except ClientError as e:
+                        error_code = e.response["Error"]["Code"]
+                        message = e.response["Error"]["Message"]
+
+                        if (
+                            error_code == "OperationAborted"
+                            and "conflicting conditional operation" in message
+                        ):
+                            # If this bucket is already being concurrently created,
+                            # we don't need to create it again.
+                            pass
+                        else:
+                            raise
+                elif error_code == "403" and message == "Forbidden":
+                    LOGGER.error(
+                        "Bucket %s exists, but access is forbidden. Please try again after "
+                        "adding appropriate access.",
+                        bucket.name,
+                    )
+                    raise
                 else:
                     raise
 

src/sagemaker/workflow/README.rst

@@ -50,10 +50,17 @@ def __init__(
         Args:
             name (str): The name of the `ModelStep`. A name is required and must be
                 unique within a pipeline.
-            step_args (_ModelStepArguments): The arguments for the `ModelStep` definition.
-            depends_on (List[Union[str, Step, StepCollection]]): A list of `Step`/`StepCollection`
-                names or `Step` instances or `StepCollection` instances that the first step,
-                in this `ModelStep` collection, depends on.
+            step_args (_ModelStepArguments): The arguments for the `ModelStep` definition,
+                generated by invoking the :func:`~sagemaker.model.Model.register` or
+                :func:`~sagemaker.model.Model.create`
+                under the :class:`~sagemaker.workflow.pipeline_context.PipelineSession`. Example::
+
+                     model = Model(sagemaker_session=PipelineSession())
+                     model_step = ModelStep(step_args=model.register())
+
+            depends_on (List[Union[str, Step, StepCollection]]):
+                A list of `Step` or `StepCollection`
+                names or `Step` instances or `StepCollection` instances that it depends on.
                 If a listed `Step` name does not exist, an error is returned (default: None).
             retry_policies (List[RetryPolicy] or Dict[str, List[RetryPolicy]]): The list of retry
                 policies for the `ModelStep` (default: None).

src/sagemaker/workflow/model_step.py

@@ -18,7 +18,6 @@
 
 import attr
 
-from sagemaker.deprecations import deprecated_class
 from sagemaker.estimator import EstimatorBase
 from sagemaker.model import Model
 from sagemaker import PipelineModel
@@ -262,18 +261,14 @@ def __init__(
         warnings.warn(
             (
                 "We are deprecating the use of RegisterModel. "
-                "Instead, please use the ModelStep, which simply takes in the step arguments "
-                "generated by model.register(). For more, see: "
+                "Please use the ModelStep instead. For more, see: "
                 "https://sagemaker.readthedocs.io/en/stable/"
                 "amazon_sagemaker_model_building_pipeline.html#model-step"
             ),
             DeprecationWarning,
         )
 
 
-RegisterModel = deprecated_class(RegisterModel, "RegisterModel")
-
-
 class EstimatorTransformer(StepCollection):
     """Creates a Transformer step collection for workflow."""
 

src/sagemaker/workflow/step_collections.py

@@ -14,7 +14,7 @@
 
 import pytest
 from botocore.exceptions import ClientError
-from mock import MagicMock
+from mock import MagicMock, patch
 import sagemaker
 
 ACCOUNT_ID = "123"
@@ -32,6 +32,11 @@ def sagemaker_session():
 
 
 def test_default_bucket_s3_create_call(sagemaker_session):
+    error = ClientError(
+        error_response={"Error": {"Code": "404", "Message": "Not Found"}},
+        operation_name="foo",
+    )
+    sagemaker_session.boto_session.resource("s3").meta.client.head_bucket.side_effect = error
     bucket_name = sagemaker_session.default_bucket()
 
     create_calls = sagemaker_session.boto_session.resource().create_bucket.mock_calls
@@ -45,6 +50,25 @@ def test_default_bucket_s3_create_call(sagemaker_session):
     assert sagemaker_session._default_bucket == bucket_name
 
 
+def test_default_bucket_s3_needs_access(sagemaker_session):
+    with patch("logging.Logger.error") as mocked_error_log:
+        with pytest.raises(ClientError):
+            error = ClientError(
+                error_response={"Error": {"Code": "403", "Message": "Forbidden"}},
+                operation_name="foo",
+            )
+            sagemaker_session.boto_session.resource(
+                "s3"
+            ).meta.client.head_bucket.side_effect = error
+            sagemaker_session.default_bucket()
+            mocked_error_log.assert_called_once_with(
+                "Bucket %s exists, but access is forbidden. Please try again after "
+                "adding appropriate access.",
+                DEFAULT_BUCKET_NAME,
+            )
+        assert sagemaker_session._default_bucket is None
+
+
 def test_default_already_cached(sagemaker_session):
     existing_default = "mydefaultbucket"
     sagemaker_session._default_bucket = existing_default
@@ -57,11 +81,9 @@ def test_default_already_cached(sagemaker_session):
 
 
 def test_default_bucket_exists(sagemaker_session):
-    error = ClientError(
-        error_response={"Error": {"Code": "BucketAlreadyOwnedByYou", "Message": "message"}},
-        operation_name="foo",
-    )
-    sagemaker_session.boto_session.resource().create_bucket.side_effect = error
+    sagemaker_session.boto_session.resource("s3").meta.client.head_bucket.return_value = {
+        "ResponseMetadata": {"RequestId": "xxx", "HTTPStatusCode": 200, "RetryAttempts": 0}
+    }
 
     bucket_name = sagemaker_session.default_bucket()
     assert bucket_name == DEFAULT_BUCKET_NAME
@@ -70,7 +92,7 @@ def test_default_bucket_exists(sagemaker_session):
 def test_concurrent_bucket_modification(sagemaker_session):
     message = "A conflicting conditional operation is currently in progress against this resource. Please try again"
     error = ClientError(
-        error_response={"Error": {"Code": "BucketAlreadyOwnedByYou", "Message": message}},
+        error_response={"Error": {"Code": "OperationAborted", "Message": message}},
         operation_name="foo",
     )
     sagemaker_session.boto_session.resource().create_bucket.side_effect = error
@@ -80,6 +102,11 @@ def test_concurrent_bucket_modification(sagemaker_session):
 
 
 def test_bucket_creation_client_error(sagemaker_session):
+    error = ClientError(
+        error_response={"Error": {"Code": "404", "Message": "Not Found"}},
+        operation_name="foo",
+    )
+    sagemaker_session.boto_session.resource("s3").meta.client.head_bucket.side_effect = error
     with pytest.raises(ClientError):
         error = ClientError(
             error_response={"Error": {"Code": "SomethingWrong", "Message": "message"}},
@@ -92,6 +119,11 @@ def test_bucket_creation_client_error(sagemaker_session):
 
 
 def test_bucket_creation_other_error(sagemaker_session):
+    error = ClientError(
+        error_response={"Error": {"Code": "404", "Message": "Not Found"}},
+        operation_name="foo",
+    )
+    sagemaker_session.boto_session.resource("s3").meta.client.head_bucket.side_effect = error
     with pytest.raises(RuntimeError):
         error = RuntimeError()
         sagemaker_session.boto_session.resource().create_bucket.side_effect = error