Repository: caikit/caikit
Pull request #591 (2023-11-29 16:13:47)

lazy_load_local_models obscures error update
fix [issue545](https://github.com/caikit/caikit/issues/545)

The`ValueError` message seen as the last statement, to provide more clarity towards the root cause of the issue.  

if `RUNTIME_LOCAL_MODELS_DIR` is not a valid path, then `RUNTIME_LAZY_LOAD_LOCAL_MODELS` set to `True` will result in the error. The existing message causes confusion at first glance, since the user has set both parameters, but the path does not exist.


Patch:
diff --git a/caikit/runtime/model_management/model_manager.py b/caikit/runtime/model_management/model_manager.py
index ac6a5ea5..a29b20f7 100644
--- a/caikit/runtime/model_management/model_manager.py
+++ b/caikit/runtime/model_management/model_manager.py
@@ -112,11 +112,26 @@ class ModelManager:  # pylint: disable=too-many-instance-attributes
 
         # Keep track of whether lazy loading is enabled
         self._lazy_load_local_models = runtime_cfg.lazy_load_local_models
-        error.value_check(
-            "<RUN44773514E>",
-            not self._lazy_load_local_models or self._local_models_dir,
-            "Must set runtime.local_models_dir with runtime.lazy_load_local_models",
-        )
+
+        if self._lazy_load_local_models:
+            error.value_check(
+                "<RUN44773525E>",
+                runtime_cfg.local_models_dir is not None,
+                (
+                    "runtime.local_models_dir must be set"
+                    " if using runtime.lazy_load_local_models. "
+                ),
+            )
+
+            error.value_check(
+                "<RUN44773514E>",
+                self._local_models_dir,
+                (
+                    "runtime.local_models_dir must be a valid path"
+                    " if set with runtime.lazy_load_local_models. "
+                    f"Provided path: {runtime_cfg.local_models_dir}"
+                ),
+            )
 
         # Set up local model periodic sync
         self._lazy_load_poll_period_seconds = runtime_cfg.lazy_load_poll_period_seconds


Test patch:
diff --git a/tests/examples/test_examples.py b/tests/examples/test_examples.py
index 8050667f..8a18ae44 100644
--- a/tests/examples/test_examples.py
+++ b/tests/examples/test_examples.py
@@ -13,7 +13,7 @@
 # limitations under the License.
 
 # Standard
-from os import getenv, path
+from os import path
 import asyncio
 import subprocess
 
diff --git a/tests/runtime/model_management/test_model_manager.py b/tests/runtime/model_management/test_model_manager.py
index be6fb557..27825da8 100644
--- a/tests/runtime/model_management/test_model_manager.py
+++ b/tests/runtime/model_management/test_model_manager.py
@@ -1075,6 +1075,129 @@ def test_lazy_load_handles_temporary_errors():
                 assert model
 
 
+def test_lazy_load_true_local_models_dir_valid():
+    """When lazy_load_local_models is True and local_models_dir exists.
+    Check that the local_models_dir is pointing to the correct location
+    """
+
+    with TemporaryDirectory() as cache_dir:
+
+        ModelManager._ModelManager__instance = None
+        with temp_config(
+            {
+                "runtime": {
+                    "local_models_dir": cache_dir,
+                    "lazy_load_local_models": True,
+                }
+            },
+            merge_strategy="merge",
+        ):
+            MODEL_MANAGER = ModelManager()
+            assert len(MODEL_MANAGER.loaded_models) == 0
+            assert MODEL_MANAGER._local_models_dir == cache_dir
+
+
+def test_lazy_load_true_local_models_dir_invalid():
+    """When lazy_load_local_models is True and local_models_dir does not exist.
+    Raise ValueError with an appropriate message
+    """
+
+    with TemporaryDirectory() as cache_dir:
+
+        with pytest.raises(
+            ValueError,
+            match=(
+                "runtime.local_models_dir must be a valid path"
+                " if set with runtime.lazy_load_local_models. "
+                "Provided path: invalid"
+            ),
+        ):
+
+            ModelManager._ModelManager__instance = None
+            with temp_config(
+                {
+                    "runtime": {
+                        "local_models_dir": "invalid",
+                        "lazy_load_local_models": True,
+                    }
+                },
+                merge_strategy="merge",
+            ):
+                MODEL_MANAGER = ModelManager()
+
+
+def test_lazy_load_true_local_models_dir_none():
+    """When lazy_load_local_models is True and local_models_dir is not set in the config.
+    Raise ValueError with an appropriate message
+    """
+
+    with TemporaryDirectory() as cache_dir:
+
+        with pytest.raises(
+            ValueError,
+            match=(
+                "runtime.local_models_dir must be set"
+                " if using runtime.lazy_load_local_models. "
+            ),
+        ):
+
+            ModelManager._ModelManager__instance = None
+            with temp_config(
+                {
+                    "runtime": {
+                        "local_models_dir": None,
+                        "lazy_load_local_models": True,
+                    }
+                },
+                merge_strategy="merge",
+            ):
+                MODEL_MANAGER = ModelManager()
+
+
+def test_lazy_load_false_local_models_dir_valid():
+    """When lazy_load_local_models is False and local_models_dir exists.
+    Check that the local_models_dir is pointing to the correct location
+    """
+
+    with TemporaryDirectory() as cache_dir:
+
+        ModelManager._ModelManager__instance = None
+        with temp_config(
+            {
+                "runtime": {
+                    "local_models_dir": cache_dir,
+                    "lazy_load_local_models": False,
+                }
+            },
+            merge_strategy="merge",
+        ):
+            MODEL_MANAGER = ModelManager()
+            assert len(MODEL_MANAGER.loaded_models) == 0
+            assert MODEL_MANAGER._local_models_dir == cache_dir
+
+
+def test_lazy_load_false_local_models_dir_invalid():
+    """When lazy_load_local_models is False and local_models_dir does not exist.
+    Check that the local_models_dir is False / Empty
+    """
+
+    with TemporaryDirectory() as cache_dir:
+
+        ModelManager._ModelManager__instance = None
+        with temp_config(
+            {
+                "runtime": {
+                    "local_models_dir": "",
+                    "lazy_load_local_models": False,
+                }
+            },
+            merge_strategy="merge",
+        ):
+            MODEL_MANAGER = ModelManager()
+            assert len(MODEL_MANAGER.loaded_models) == 0
+            assert not MODEL_MANAGER._local_models_dir
+
+
 class NoModelFinder(ModelFinderBase):
     name = "NOMODEL"