Merge branch 'main' into ODSC-66852/watch_aqua_ft_job

Author: VipulMascarenhas

ads/aqua/app.py

@@ -4,6 +4,7 @@
 
 import json
 import os
+import traceback
 from dataclasses import fields
 from typing import Dict, Union
 
@@ -23,7 +24,7 @@
 from ads.aqua.constants import UNKNOWN
 from ads.common import oci_client as oc
 from ads.common.auth import default_signer
-from ads.common.utils import extract_region
+from ads.common.utils import extract_region, is_path_exists
 from ads.config import (
     AQUA_TELEMETRY_BUCKET,
     AQUA_TELEMETRY_BUCKET_NS,
@@ -296,33 +297,44 @@ def get_config(self, model_id: str, config_file_name: str) -> Dict:
             raise AquaRuntimeError(f"Target model {oci_model.id} is not Aqua model.")
 
         config = {}
-        artifact_path = get_artifact_path(oci_model.custom_metadata_list)
+        # if the current model has a service model tag, then
+        if Tags.AQUA_SERVICE_MODEL_TAG in oci_model.freeform_tags:
+            base_model_ocid = oci_model.freeform_tags[Tags.AQUA_SERVICE_MODEL_TAG]
+            logger.info(
+                f"Base model found for the model: {oci_model.id}. "
+                f"Loading {config_file_name} for base model {base_model_ocid}."
+            )
+            base_model = self.ds_client.get_model(base_model_ocid).data
+            artifact_path = get_artifact_path(base_model.custom_metadata_list)
+            config_path = f"{os.path.dirname(artifact_path)}/config/"
+        else:
+            logger.info(f"Loading {config_file_name} for model {oci_model.id}...")
+            artifact_path = get_artifact_path(oci_model.custom_metadata_list)
+            config_path = f"{artifact_path.rstrip('/')}/config/"
+
         if not artifact_path:
             logger.debug(
                 f"Failed to get artifact path from custom metadata for the model: {model_id}"
             )
             return config
 
-        try:
-            config_path = f"{os.path.dirname(artifact_path)}/config/"
-            config = load_config(
-                config_path,
-                config_file_name=config_file_name,
-            )
-        except Exception:
-            # todo: temp fix for issue related to config load for byom models, update logic to choose the right path
+        config_file_path = f"{config_path}{config_file_name}"
+        if is_path_exists(config_file_path):
             try:
-                config_path = f"{artifact_path.rstrip('/')}/config/"
                 config = load_config(
                     config_path,
                     config_file_name=config_file_name,
                 )
             except Exception:
-                pass
+                logger.debug(
+                    f"Error loading the {config_file_name} at path {config_path}.\n"
+                    f"{traceback.format_exc()}"
+                )
 
         if not config:
-            logger.error(
-                f"{config_file_name} is not available for the model: {model_id}. Check if the custom metadata has the artifact path set."
+            logger.debug(
+                f"{config_file_name} is not available for the model: {model_id}. "
+                f"Check if the custom metadata has the artifact path set."
             )
             return config
 

ads/aqua/model/model.py

@@ -29,7 +29,6 @@
     LifecycleStatus,
     _build_resource_identifier,
     cleanup_local_hf_model_artifact,
-    copy_model_config,
     create_word_icon,
     generate_tei_cmd_var,
     get_artifact_path,
@@ -969,24 +968,6 @@ def _create_model_catalog_entry(
                 )
                 tags[Tags.LICENSE] = validation_result.tags.get(Tags.LICENSE, UNKNOWN)
 
-        try:
-            # If verified model already has a artifact json, use that.
-            artifact_path = metadata.get(MODEL_BY_REFERENCE_OSS_PATH_KEY).value
-            logger.info(
-                f"Found model artifact in the service bucket. "
-                f"Using artifact from service bucket instead of {os_path}."
-            )
-
-            # todo: implement generic copy_folder method
-            # copy model config from artifact path to user bucket
-            copy_model_config(
-                artifact_path=artifact_path, os_path=os_path, auth=default_signer()
-            )
-        except Exception:
-            logger.debug(
-                f"Proceeding with model registration without copying model config files at {os_path}. "
-                f"Default configuration will be used for deployment and fine-tuning."
-            )
         # Set artifact location to user bucket, and replace existing key if present.
         metadata.add(
             key=MODEL_BY_REFERENCE_OSS_PATH_KEY,

tests/unitary/with_extras/aqua/test_config.py

@@ -1,13 +1,17 @@
 #!/usr/bin/env python
-# Copyright (c) 2024 Oracle and/or its affiliates.
+# Copyright (c) 2024, 2025 Oracle and/or its affiliates.
 # Licensed under the Universal Permissive License v 1.0 as shown at https://oss.oracle.com/licenses/upl/
 
 import json
 import os
-from unittest.mock import patch
+import pytest
+from unittest.mock import patch, MagicMock
+
+import oci.data_science.models
 
 from ads.aqua.common.entities import ContainerSpec
 from ads.aqua.config.config import get_evaluation_service_config
+from ads.aqua.app import AquaApp
 
 
 class TestConfig:
@@ -37,3 +41,63 @@ def test_evaluation_service_config(self, mock_get_container_config):
             test_result.to_dict()
             == expected_result[ContainerSpec.CONTAINER_SPEC]["test_container"]
         )
+
+    @pytest.mark.parametrize(
+        "custom_metadata",
+        [
+            {
+                "category": "Other",
+                "description": "test_desc",
+                "key": "artifact_location",
+                "value": "artifact_location",
+            },
+            {},
+        ],
+    )
+    @pytest.mark.parametrize("verified_model", [True, False])
+    @pytest.mark.parametrize("path_exists", [True, False])
+    @patch("ads.aqua.app.load_config")
+    def test_load_config(
+        self, mock_load_config, custom_metadata, verified_model, path_exists
+    ):
+        mock_load_config.return_value = {"config_key": "config_value"}
+        service_model_tag = (
+            {"aqua_service_model": "aqua_service_model_id"} if verified_model else {}
+        )
+
+        self.app = AquaApp()
+
+        model = {
+            "id": "mock_id",
+            "lifecycle_details": "mock_lifecycle_details",
+            "lifecycle_state": "mock_lifecycle_state",
+            "project_id": "mock_project_id",
+            "freeform_tags": {
+                **{
+                    "OCI_AQUA": "",
+                },
+                **service_model_tag,
+            },
+            "custom_metadata_list": [
+                oci.data_science.models.Metadata(**custom_metadata)
+            ],
+        }
+
+        self.app.ds_client.get_model = MagicMock(
+            return_value=oci.response.Response(
+                status=200,
+                request=MagicMock(),
+                headers=MagicMock(),
+                data=oci.data_science.models.Model(**model),
+            )
+        )
+        with patch("ads.aqua.app.is_path_exists", return_value=path_exists):
+            result = self.app.get_config(
+                model_id="test_model_id", config_file_name="test_config_file_name"
+            )
+            if not path_exists:
+                assert result == {}
+            if not custom_metadata:
+                assert result == {}
+            if path_exists and custom_metadata:
+                assert result == {"config_key": "config_value"}

tests/unitary/with_extras/aqua/test_model.py

@@ -665,7 +665,6 @@ def test_get_model_fine_tuned(
     @patch("ads.model.datascience_model.DataScienceModel.sync")
     @patch("ads.model.datascience_model.DataScienceModel.upload_artifact")
     @patch.object(AquaModelApp, "_find_matching_aqua_model")
-    @patch("ads.aqua.common.utils.copy_file")
     @patch("ads.common.object_storage_details.ObjectStorageDetails.list_objects")
     @patch("ads.aqua.common.utils.load_config", return_value={})
     @patch("huggingface_hub.snapshot_download")
@@ -676,7 +675,6 @@ def test_import_verified_model(
         mock_snapshot_download,
         mock_load_config,
         mock_list_objects,
-        mock_copy_file,
         mock__find_matching_aqua_model,
         mock_upload_artifact,
         mock_sync,
@@ -788,8 +786,6 @@ def test_import_verified_model(
             mock_subprocess.assert_not_called()
             mock_load_config.assert_called()
 
-        if not artifact_location_set:
-            mock_copy_file.assert_called()
         ds_freeform_tags.pop(
             "ready_to_import"
         )  # The imported model should not have this tag