Merge branch 'main' of https://github.com/oracle/accelerated-data-science into ODSC-47079/fixed_pipeline_step_to_dict

lu-ohai · lu-ohai · commit 4eb2deb32a90 · 2023-09-14T17:02:26.000-04:00
diff --git a/ads/model/deployment/model_deployment.py b/ads/model/deployment/model_deployment.py
@@ -1304,7 +1304,8 @@ def from_id(cls, id: str) -> "ModelDeployment":
         ModelDeployment
             The ModelDeployment instance (self).
         """
-        return cls()._update_from_oci_model(OCIDataScienceModelDeployment.from_id(id))
+        oci_model = OCIDataScienceModelDeployment.from_id(id)
+        return cls(properties=oci_model)._update_from_oci_model(oci_model)
 
     @classmethod
     def from_dict(cls, obj_dict: Dict) -> "ModelDeployment":
@@ -1503,7 +1504,9 @@ def _build_model_deployment_details(self) -> CreateModelDeploymentDetails:
             **create_model_deployment_details
         ).to_oci_model(CreateModelDeploymentDetails)
 
-    def _update_model_deployment_details(self, **kwargs) -> UpdateModelDeploymentDetails:
+    def _update_model_deployment_details(
+        self, **kwargs
+    ) -> UpdateModelDeploymentDetails:
         """Builds UpdateModelDeploymentDetails from model deployment instance.
 
         Returns
@@ -1527,7 +1530,7 @@ def _update_model_deployment_details(self, **kwargs) -> UpdateModelDeploymentDet
         return OCIDataScienceModelDeployment(
             **update_model_deployment_details
         ).to_oci_model(UpdateModelDeploymentDetails)
-    
+
     def _update_spec(self, **kwargs) -> "ModelDeployment":
         """Updates model deployment specs from kwargs.
 
@@ -1542,7 +1545,7 @@ def _update_spec(self, **kwargs) -> "ModelDeployment":
                 Model deployment freeform tags
             defined_tags: (dict)
                 Model deployment defined tags
-            
+
             Additional kwargs arguments.
             Can be any attribute that `ads.model.deployment.ModelDeploymentCondaRuntime`, `ads.model.deployment.ModelDeploymentContainerRuntime`
             and `ads.model.deployment.ModelDeploymentInfrastructure` accepts.
@@ -1559,20 +1562,22 @@ def _update_spec(self, **kwargs) -> "ModelDeployment":
         specs = {
             "self": self._spec,
             "runtime": self.runtime._spec,
-            "infrastructure": self.infrastructure._spec
+            "infrastructure": self.infrastructure._spec,
         }
         sub_set = {
             self.infrastructure.CONST_ACCESS_LOG,
             self.infrastructure.CONST_PREDICT_LOG,
-            self.infrastructure.CONST_SHAPE_CONFIG_DETAILS
+            self.infrastructure.CONST_SHAPE_CONFIG_DETAILS,
         }
         for spec_value in specs.values():
             for key in spec_value:
                 if key in converted_specs:
                     if key in sub_set:
                         for sub_key in converted_specs[key]:
                             converted_sub_key = ads_utils.snake_to_camel(sub_key)
-                            spec_value[key][converted_sub_key] = converted_specs[key][sub_key]
+                            spec_value[key][converted_sub_key] = converted_specs[key][
+                                sub_key
+                            ]
                     else:
                         spec_value[key] = copy.deepcopy(converted_specs[key])
         self = (
@@ -1616,14 +1621,14 @@ def _build_model_deployment_configuration_details(self) -> Dict:
                 infrastructure.CONST_MEMORY_IN_GBS: infrastructure.shape_config_details.get(
                     "memory_in_gbs", None
                 )
-                or infrastructure.shape_config_details.get(
-                    "memoryInGBs", None
-                )
+                or infrastructure.shape_config_details.get("memoryInGBs", None)
                 or DEFAULT_MEMORY_IN_GBS,
             }
 
         if infrastructure.subnet_id:
-            instance_configuration[infrastructure.CONST_SUBNET_ID] = infrastructure.subnet_id
+            instance_configuration[
+                infrastructure.CONST_SUBNET_ID
+            ] = infrastructure.subnet_id
 
         scaling_policy = {
             infrastructure.CONST_POLICY_TYPE: "FIXED_SIZE",
@@ -1638,13 +1643,11 @@ def _build_model_deployment_configuration_details(self) -> Dict:
 
         model_id = runtime.model_uri
         if not model_id.startswith("ocid"):
-            
             from ads.model.datascience_model import DataScienceModel
-            
+
             dsc_model = DataScienceModel(
                 name=self.display_name,
-                compartment_id=self.infrastructure.compartment_id
-                or COMPARTMENT_OCID,
+                compartment_id=self.infrastructure.compartment_id or COMPARTMENT_OCID,
                 project_id=self.infrastructure.project_id or PROJECT_OCID,
                 artifact=runtime.model_uri,
             ).create(
@@ -1653,7 +1656,7 @@ def _build_model_deployment_configuration_details(self) -> Dict:
                 region=runtime.region,
                 overwrite_existing_artifact=runtime.overwrite_existing_artifact,
                 remove_existing_artifact=runtime.remove_existing_artifact,
-                timeout=runtime.timeout
+                timeout=runtime.timeout,
             )
             model_id = dsc_model.id
 
diff --git a/ads/model/service/oci_datascience_model.py b/ads/model/service/oci_datascience_model.py
@@ -38,19 +38,19 @@
 )
 
 
-class ModelProvenanceNotFoundError(Exception):   # pragma: no cover
+class ModelProvenanceNotFoundError(Exception):  # pragma: no cover
     pass
 
 
-class ModelArtifactNotFoundError(Exception):   # pragma: no cover
+class ModelArtifactNotFoundError(Exception):  # pragma: no cover
     pass
 
 
-class ModelNotSavedError(Exception):   # pragma: no cover
+class ModelNotSavedError(Exception):  # pragma: no cover
     pass
 
 
-class ModelWithActiveDeploymentError(Exception):   # pragma: no cover
+class ModelWithActiveDeploymentError(Exception):  # pragma: no cover
     pass
 
 
@@ -410,7 +410,7 @@ def export_model_artifact(self, bucket_uri: str, region: str = None):
         # Show progress of exporting model artifacts
         self._wait_for_work_request(
             work_request_id=work_request_id,
-            num_steps=3,
+            num_steps=2,
         )
 
     @check_for_model_id(
@@ -596,3 +596,7 @@ def _wait_for_work_request(self, work_request_id: str, num_steps: int = 3) -> No
                             )
                     else:
                         break
+
+            while i < num_steps:
+                progress.update()
+                i += 1
diff --git a/docs/source/user_guide/model_catalog/model_catalog.rst b/docs/source/user_guide/model_catalog/model_catalog.rst
@@ -1204,7 +1204,7 @@ If you don't have an Object Storage bucket, create one using the OCI SDK or the
 
         Allow service datascience to manage object-family in compartment <compartment> where ALL {target.bucket.name='<bucket_name>'}
 
-        Allow service objectstorage to manage object-family in compartment <compartment> where ALL {target.bucket.name='<bucket_name>'}
+        Allow service objectstorage-<region> to manage object-family in compartment <compartment> where ALL {target.bucket.name='<bucket_name>'}
 
 Saving
 ======
@@ -1545,4 +1545,3 @@ In the next example, the model that was stored in the model catalog as part of t
 .. code-block:: python3
 
     mc.delete_model(mc_model.id)
-
diff --git a/docs/source/user_guide/model_registration/large_model_artifact.rst b/docs/source/user_guide/model_registration/large_model_artifact.rst
@@ -13,7 +13,7 @@ If you don't have an Object Storage bucket, create one using the OCI SDK or the
 
         Allow service datascience to manage object-family in compartment <compartment> where ALL {target.bucket.name='<bucket_name>'}
 
-        Allow service objectstorage to manage object-family in compartment <compartment> where ALL {target.bucket.name='<bucket_name>'}
+        Allow service objectstorage-<region> to manage object-family in compartment <compartment> where ALL {target.bucket.name='<bucket_name>'}
 
 See `API documentation <../../ads.model.html#id10>`__ for more details.
 
diff --git a/docs/source/user_guide/model_registration/model_load.rst b/docs/source/user_guide/model_registration/model_load.rst
@@ -119,7 +119,7 @@ If you don't have an Object Storage bucket, create one using the OCI SDK or the
 
         Allow service datascience to manage object-family in compartment <compartment> where ALL {target.bucket.name='<bucket_name>'}
 
-        Allow service objectstorage to manage object-family in compartment <compartment> where ALL {target.bucket.name='<bucket_name>'}
+        Allow service objectstorage-<region> to manage object-family in compartment <compartment> where ALL {target.bucket.name='<bucket_name>'}
 
 The following example loads a model using the large model artifact approach. The ``bucket_uri`` has the following syntax: ``oci://<bucket_name>@<namespace>/<path>/`` See `API documentation <../../ads.model.html#id4>`__ for more details.
 
@@ -169,4 +169,4 @@ Alternatively the ``.from_id()`` method can be used to load registered or deploy
         bucket_uri=<oci://<bucket_name>@<namespace>/prefix/>,
         force_overwrite=True,
         remove_existing_artifact=True,
-    )
+    )
diff --git a/tests/unitary/default_setup/model/test_oci_datascience_model.py b/tests/unitary/default_setup/model/test_oci_datascience_model.py
@@ -88,7 +88,6 @@
 
 class TestOCIDataScienceModel:
     def setup_class(cls):
-
         # Mock delete model response
         cls.mock_delete_model_response = Response(
             data=None, status=None, headers=None, request=None
@@ -229,7 +228,9 @@ def test_delete_success(self, mock_client):
                 mock_model_deployment.return_value = [
                     MagicMock(lifecycle_state="ACTIVE", identifier="md_id")
                 ]
-                with patch("ads.model.deployment.ModelDeployment.from_id") as mock_from_id:
+                with patch(
+                    "ads.model.deployment.ModelDeployment.from_id"
+                ) as mock_from_id:
                     with patch.object(OCIDataScienceModel, "sync") as mock_sync:
                         self.mock_model.delete(delete_associated_model_deployment=True)
                         mock_from_id.assert_called_with("md_id")
@@ -445,7 +446,7 @@ def test_export_model_artifact(
             )
             mock_wait_for_work_request.assert_called_with(
                 work_request_id="work_request_id",
-                num_steps=3,
+                num_steps=2,
             )
 
     @patch.object(TqdmProgressBar, "update")
