Merge branch 'main' into ODSC-51216/cleanup_summary_status_updates

VipulMascarenhas · VipulMascarenhas · commit 2cd078a75688 · 2024-01-04T16:32:54.000-08:00
diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml
@@ -1,52 +1,52 @@
 repos:
-# Standard hooks
--   repo: https://github.com/pre-commit/pre-commit-hooks
-    rev: v4.4.0
-    hooks:
-    -   id: check-ast
-        exclude: ^docs/
-    -   id: check-docstring-first
-        exclude: ^(docs/|tests/)
-    -   id: check-json
-    -   id: check-merge-conflict
-    -   id: check-yaml
-        exclude: feature_store_*.yaml
-        args: ['--allow-multiple-documents']
-    -   id: detect-private-key
-    -   id: end-of-file-fixer
-        exclude: '\.ipynb?$'
-    -   id: pretty-format-json
-        args: ['--autofix']
-    -   id: trailing-whitespace
-        args: [--markdown-linebreak-ext=md]
-        exclude: ^docs/
-# Black, the code formatter, natively supports pre-commit
--   repo: https://github.com/psf/black
-    rev: 23.3.0
-    hooks:
-    -   id: black
-        exclude: ^docs/
-# Regex based rst files common mistakes detector
--   repo: https://github.com/pre-commit/pygrep-hooks
-    rev: v1.10.0
-    hooks:
-    -   id: rst-backticks
-        files: ^docs/
-    -   id: rst-inline-touching-normal
-        files: ^docs/
-# Hardcoded secrets and ocids detector
--   repo: https://github.com/gitleaks/gitleaks
-    rev: v8.17.0
-    hooks:
-    -   id: gitleaks
-        exclude: .github/workflows/reusable-actions/set-dummy-conf.yml
-# Oracle copyright checker
--   repo: https://github.com/oracle-samples/oci-data-science-ai-samples/
-    rev: cbe0136f7aaffe463b31ddf3f34b0e16b4b124ff
-    hooks:
-    -   id: check-copyright
-        name: check-copyright
-        entry: .pre-commit-scripts/check-copyright.py
-        language: script
-        types_or: ['python', 'shell', 'bash']
-        exclude: ^docs/
+    # Standard hooks
+    - repo: https://github.com/pre-commit/pre-commit-hooks
+      rev: v4.4.0
+      hooks:
+          - id: check-ast
+            exclude: ^docs/
+          - id: check-docstring-first
+            exclude: ^(docs/|tests/)
+          - id: check-json
+          - id: check-merge-conflict
+          - id: check-yaml
+            exclude: feature_store_*.yaml
+            args: ["--allow-multiple-documents"]
+          - id: detect-private-key
+          - id: end-of-file-fixer
+            exclude: '\.ipynb?$'
+          - id: pretty-format-json
+            args: ["--autofix"]
+          - id: trailing-whitespace
+            args: [--markdown-linebreak-ext=md]
+            exclude: ^docs/
+    # Black, the code formatter, natively supports pre-commit
+    - repo: https://github.com/psf/black
+      rev: 23.3.0
+      hooks:
+          - id: black
+            exclude: ^docs/
+    # Regex based rst files common mistakes detector
+    - repo: https://github.com/pre-commit/pygrep-hooks
+      rev: v1.10.0
+      hooks:
+          - id: rst-backticks
+            files: ^docs/
+          - id: rst-inline-touching-normal
+            files: ^docs/
+    # Hardcoded secrets and ocids detector
+    - repo: https://github.com/gitleaks/gitleaks
+      rev: v8.17.0
+      hooks:
+          - id: gitleaks
+            exclude: .github/workflows/reusable-actions/set-dummy-conf.yml
+    # Oracle copyright checker
+    - repo: https://github.com/oracle-samples/oci-data-science-ai-samples/
+      rev: 1bc5270a443b791c62f634233c0f4966dfcc0dd6
+      hooks:
+          - id: check-copyright
+            name: check-copyright
+            entry: .pre-commit-scripts/check-copyright.py
+            language: script
+            types_or: ["python", "shell", "bash"]
+            exclude: ^docs/
diff --git a/ads/common/dsc_file_system.py b/ads/common/dsc_file_system.py
@@ -265,8 +265,6 @@ def update_from_dsc_model(cls, dsc_model) -> dict:
 
 class DSCFileSystemManager:
 
-    storage_mount_dest = set()
-
     @classmethod
     def initialize(cls, arguments: dict) -> dict:
         """Initialize and update arguments to dsc model.
@@ -286,12 +284,6 @@ def initialize(cls, arguments: dict) -> dict:
                 "Parameter `dest` is required for mounting file storage system."
             )
 
-        if arguments["dest"] in cls.storage_mount_dest:
-            raise ValueError(
-                "Duplicate `dest` found. Please specify different `dest` for each file system to be mounted."
-            )
-        cls.storage_mount_dest.add(arguments["dest"])
-
         # case oci://bucket@namespace/prefix
         if arguments["src"].startswith("oci://") and "@" in arguments["src"]:
             return OCIObjectStorage(**arguments).update_to_dsc_model()
diff --git a/ads/jobs/builders/runtimes/container_runtime.py b/ads/jobs/builders/runtimes/container_runtime.py
@@ -23,7 +23,7 @@ class ContainerRuntime(Runtime):
     >>> ContainerRuntime()
     >>> .with_image(
     >>>     "iad.ocir.io/<your_tenancy>/<your_image>",
-    >>>     entrypoint=["/bin/sh", -c],
+    >>>     entrypoint=["/bin/sh", "-c"],
     >>>     cmd="sleep 5 && echo Hello World",
     >>> )
     >>> .with_environment_variable(MY_ENV="MY_VALUE")
diff --git a/ads/llm/chain.py b/ads/llm/chain.py
@@ -261,12 +261,8 @@ def load(cls, chain_dict: dict, **kwargs) -> "GuardrailSequence":
         from ads.llm.serialize import load
 
         chain_spec = chain_dict[SPEC_CHAIN]
-        chain = cls()
-        for config in chain_spec:
-            step = load(config, **kwargs)
-            # Chain the step
-            chain |= step
-        return chain
+        steps = [load(config, **kwargs) for config in chain_spec]
+        return cls(*steps)
 
     def __str__(self) -> str:
         return "\n".join([str(step.__class__) for step in self.steps])
diff --git a/ads/model/artifact.py b/ads/model/artifact.py
@@ -30,21 +30,21 @@
 ADS_VERSION = __version__
 
 
-class ArtifactNestedFolderError(Exception):   # pragma: no cover
+class ArtifactNestedFolderError(Exception):  # pragma: no cover
     def __init__(self, folder: str):
         self.folder = folder
         super().__init__("The required artifact files placed in a nested folder.")
 
 
-class ArtifactRequiredFilesError(Exception):   # pragma: no cover
+class ArtifactRequiredFilesError(Exception):  # pragma: no cover
     def __init__(self, required_files: Tuple[str]):
         super().__init__(
             "Not all required files presented in artifact folder. "
             f"Required files for conda runtime: {required_files}. If you are using container runtime, set `ignore_conda_error=True`."
         )
 
 
-class AritfactFolderStructureError(Exception):   # pragma: no cover
+class AritfactFolderStructureError(Exception):  # pragma: no cover
     def __init__(self, required_files: Tuple[str]):
         super().__init__(
             "The artifact folder has a wrong structure. "
@@ -171,6 +171,7 @@ def __init__(
         self.ignore_conda_error = ignore_conda_error
         self.model = None
         self.auth = auth or authutil.default_signer()
+
         if reload and not ignore_conda_error:
             self.reload()
             # Extracts the model_file_name from the score.py.
@@ -272,8 +273,9 @@ def prepare_runtime_yaml(
             or runtime_info.model_deployment.inference_conda_env.inference_python_version.strip()
             == ""
         ):
-            warnings.warn(
-                "Cannot automatically detect the inference python version. `inference_python_version` must be provided."
+            raise ValueError(
+                "Cannot automatically detect the inference python version. "
+                "`inference_python_version` must be provided."
             )
         runtime_file_path = os.path.join(self.artifact_dir, "runtime.yaml")
         if os.path.exists(runtime_file_path) and not force_overwrite:
@@ -416,6 +418,7 @@ def from_uri(
         force_overwrite: Optional[bool] = False,
         auth: Optional[Dict] = None,
         ignore_conda_error: Optional[bool] = False,
+        reload: Optional[bool] = False,
     ):
         """Constructs a ModelArtifact object from the existing model artifacts.
 
@@ -426,16 +429,20 @@ def from_uri(
             OCI object storage URI.
         artifact_dir: str
             The local artifact folder to store the files needed for deployment.
-        model_file_name: (str, optional). Defaults to `None`
-            The file name of the serialized model.
-        force_overwrite: (bool, optional). Defaults to False.
-            Whether to overwrite existing files or not.
         auth: (Dict, optional). Defaults to None.
             The default authetication is set using `ads.set_auth` API.
             If you need to override the default, use the `ads.common.auth.api_keys`
             or `ads.common.auth.resource_principal` to create appropriate
             authentication signer and kwargs required to instantiate
             IdentityClient object.
+        force_overwrite: (bool, optional). Defaults to False.
+            Whether to overwrite existing files or not.
+        ignore_conda_error: (bool, optional). Defaults to False.
+            Parameter to ignore error when collecting conda information.
+        model_file_name: (str, optional). Defaults to `None`
+            The file name of the serialized model.
+        reload: (bool, optional). Defaults to False.
+            Whether to reload the Model into the environment.
 
         Returns
         -------
@@ -492,6 +499,8 @@ def from_uri(
                     utils.copy_from_uri(
                         uri=temp_dir, to_path=to_path, force_overwrite=True
                     )
+            except ArtifactRequiredFilesError as ex:
+                logger.warning(ex)
 
         if ObjectStorageDetails.is_oci_path(artifact_dir):
             for root, dirs, files in os.walk(to_path):
@@ -507,10 +516,10 @@ def from_uri(
 
         return cls(
             artifact_dir=artifact_dir,
-            model_file_name=model_file_name,
-            reload=True,
             ignore_conda_error=ignore_conda_error,
             local_copy_dir=to_path,
+            model_file_name=model_file_name,
+            reload=reload,
         )
 
     def __getattr__(self, item):
diff --git a/ads/model/generic_model.py b/ads/model/generic_model.py
@@ -975,17 +975,20 @@ def prepare(
             auth=self.auth,
             local_copy_dir=self.local_copy_dir,
         )
-        self.runtime_info = self.model_artifact.prepare_runtime_yaml(
-            inference_conda_env=self.properties.inference_conda_env,
-            inference_python_version=self.properties.inference_python_version,
-            training_conda_env=self.properties.training_conda_env,
-            training_python_version=self.properties.training_python_version,
-            force_overwrite=force_overwrite,
-            namespace=namespace,
-            bucketname=DEFAULT_CONDA_BUCKET_NAME,
-            auth=self.auth,
-            ignore_conda_error=self.ignore_conda_error,
-        )
+        try:
+            self.runtime_info = self.model_artifact.prepare_runtime_yaml(
+                inference_conda_env=self.properties.inference_conda_env,
+                inference_python_version=self.properties.inference_python_version,
+                training_conda_env=self.properties.training_conda_env,
+                training_python_version=self.properties.training_python_version,
+                force_overwrite=force_overwrite,
+                namespace=namespace,
+                bucketname=DEFAULT_CONDA_BUCKET_NAME,
+                auth=self.auth,
+                ignore_conda_error=self.ignore_conda_error,
+            )
+        except ValueError as e:
+            raise e
 
         self._summary_status.update_status(
             detail="Generated runtime.yaml", status=ModelState.DONE.value
@@ -1361,13 +1364,15 @@ def from_model_artifact(
         properties.with_dict(local_vars)
         auth = auth or authutil.default_signer()
         artifact_dir = _prepare_artifact_dir(artifact_dir)
+        reload = kwargs.pop("reload", False)
         model_artifact = ModelArtifact.from_uri(
             uri=uri,
             artifact_dir=artifact_dir,
-            model_file_name=model_file_name,
-            force_overwrite=force_overwrite,
             auth=auth,
+            force_overwrite=force_overwrite,
             ignore_conda_error=ignore_conda_error,
+            model_file_name=model_file_name,
+            reload=reload,
         )
         model = cls(
             estimator=model_artifact.model,
@@ -1380,22 +1385,33 @@ def from_model_artifact(
         model.local_copy_dir = model_artifact.local_copy_dir
         model.model_artifact = model_artifact
         model.ignore_conda_error = ignore_conda_error
-        model.reload_runtime_info()
+
+        if reload:
+            model.reload_runtime_info()
+            model._summary_status.update_action(
+                detail="Populated metadata(Custom, Taxonomy and Provenance)",
+                action="Call .populate_metadata() to populate metadata.",
+            )
+
         model._summary_status.update_status(
             detail="Generated score.py",
-            status=ModelState.DONE.value,
+            status=ModelState.NOTAPPLICABLE.value,
         )
         model._summary_status.update_status(
             detail="Generated runtime.yaml",
-            status=ModelState.DONE.value,
+            status=ModelState.NOTAPPLICABLE.value,
         )
         model._summary_status.update_status(
-            detail="Serialized model", status=ModelState.DONE.value
+            detail="Serialized model",
+            status=ModelState.NOTAPPLICABLE.value,
         )
-        model._summary_status.update_action(
+        model._summary_status.update_status(
             detail="Populated metadata(Custom, Taxonomy and Provenance)",
-            action=f"Call .populate_metadata() to populate metadata.",
+            status=ModelState.AVAILABLE.value
+            if reload
+            else ModelState.NOTAPPLICABLE.value,
         )
+
         return model
 
     def download_artifact(
diff --git a/ads/model/runtime/env_info.py b/ads/model/runtime/env_info.py
@@ -106,7 +106,7 @@ def from_slug(
                 env_path, python_version = service_pack_slug_mapping[env_slug]
             else:
                 warnings.warn(
-                    "The {env_slug} is not a service pack. Use `from_path` method by passing in the object storage path."
+                    f"The {env_slug} is not a service pack. Use `from_path` method by passing in the object storage path."
                 )
 
         return cls._populate_env_info(
@@ -157,9 +157,13 @@ def from_path(cls, env_path: str, auth: dict = None) -> "EnvInfo":
                     ).fetch_metadata_of_object()
                     python_version = metadata_json.get("python", None)
                     env_slug = metadata_json.get("slug", None)
+                    if not python_version:
+                        raise ValueError(
+                            f"The manifest metadata of {env_path} doesn't contains inforamtion for python version."
+                        )
                 except Exception as e:
-                    logging.warning(e)
-                    logging.warning(
+                    logging.debug(e)
+                    logging.debug(
                         "python version and slug are not found from the manifest metadata."
                     )
 
@@ -227,7 +231,7 @@ def _populate_env_info(
         )
 
     @classmethod
-    def _validate_dict(cls,obj_dict: Dict) -> bool:
+    def _validate_dict(cls, obj_dict: Dict) -> bool:
         """Validate the content in the dictionary format from the yaml file.
 
         Parameters
diff --git a/docs/source/user_guide/jobs/data_science_job.rst b/docs/source/user_guide/jobs/data_science_job.rst
@@ -85,6 +85,10 @@ to create the job on OCI. To start a job run, you can call the :py:meth:`~ads.jo
 which returns a :py:class:`~ads.jobs.DataScienceJobRun` instance.
 Once the job or job run is created, the job OCID can be accessed through ``job.id`` or ``run.id``.
 
+.. note::
+
+  Once a job is created, if you change the configuration, you will need to re-create a job for the new configuration.
+
 .. code-block:: python
 
   # Create the job on OCI Data Science
diff --git a/tests/unitary/default_setup/jobs/test_jobs_mount_file_system.py b/tests/unitary/default_setup/jobs/test_jobs_mount_file_system.py
@@ -350,14 +350,6 @@ def test_file_manager_process_data_error(self):
         ):
             DSCFileSystemManager.initialize(test_mount_file_system)
 
-        test_mount_file_system_list = [test_mount_file_system] * 2
-        with pytest.raises(
-            ValueError,
-            match="Duplicate `dest` found. Please specify different `dest` for each file system to be mounted."
-        ):
-            for mount_file_system in test_mount_file_system_list:
-                DSCFileSystemManager.initialize(mount_file_system)
-
     def test_dsc_object_storage(self):
         object_storage = OCIObjectStorage(
             src="oci://bucket@namespace/prefix",
diff --git a/tests/unitary/with_extras/langchain/test_guardrails.py b/tests/unitary/with_extras/langchain/test_guardrails.py
diff --git a/tests/unitary/with_extras/langchain/test_serialization.py b/tests/unitary/with_extras/langchain/test_serialization.py
diff --git a/tests/unitary/with_extras/model/test_generic_model.py b/tests/unitary/with_extras/model/test_generic_model.py
