From e8cd8508b5e6f819f186f26da583690caec7537b Mon Sep 17 00:00:00 2001 From: Doug Edgar Date: Thu, 30 Oct 2025 17:01:31 -0700 Subject: [PATCH] fix: handle missing external_providers_dir (#3974) # What does this PR do? This PR fixes the handling of the external_providers_dir configuration field to align with its ongoing deprecation, in favor of the provider `module` specification approach. It addresses the issue in #3950, where using the default provided run.yaml config resulted in the `external_providers_dir` parameter being set to the literal string `None`, and crashing the llama-stack server when starting. Closes #3950 ## Test Plan - Built a new container image from `podman build . -f containers/Containerfile --build-arg DISTRO_NAME=starter --tag llama-stack:starter` - Tested it locally with `podman run -it localhost/llama-stack:starter` - Tested it on an OpenShift 4.19 cluster, deployed via the llama-stack-k8s-operator. Signed-off-by: Doug Edgar --- src/llama_stack/cli/stack/run.py | 3 ++- src/llama_stack/core/configure.py | 9 --------- tests/unit/cli/test_stack_config.py | 23 +++++++++++++++++++++++ 3 files changed, 25 insertions(+), 10 deletions(-) diff --git a/src/llama_stack/cli/stack/run.py b/src/llama_stack/cli/stack/run.py index 044ce49c9..c9334b9e9 100644 --- a/src/llama_stack/cli/stack/run.py +++ b/src/llama_stack/cli/stack/run.py @@ -106,7 +106,8 @@ class StackRun(Subcommand): try: config = parse_and_maybe_upgrade_config(config_dict) - if not os.path.exists(str(config.external_providers_dir)): + # Create external_providers_dir if it's specified and doesn't exist + if config.external_providers_dir and not os.path.exists(str(config.external_providers_dir)): os.makedirs(str(config.external_providers_dir), exist_ok=True) except AttributeError as e: self.parser.error(f"failed to parse config file '{config_file}':\n {e}") diff --git a/src/llama_stack/core/configure.py b/src/llama_stack/core/configure.py index 734839ea9..5d4a54184 100644 --- a/src/llama_stack/core/configure.py +++ b/src/llama_stack/core/configure.py @@ -17,7 +17,6 @@ from llama_stack.core.distribution import ( get_provider_registry, ) from llama_stack.core.stack import cast_image_name_to_string, replace_env_vars -from llama_stack.core.utils.config_dirs import EXTERNAL_PROVIDERS_DIR from llama_stack.core.utils.dynamic import instantiate_class_type from llama_stack.core.utils.prompt_for_config import prompt_for_config from llama_stack.log import get_logger @@ -194,19 +193,11 @@ def upgrade_from_routing_table( def parse_and_maybe_upgrade_config(config_dict: dict[str, Any]) -> StackRunConfig: - version = config_dict.get("version", None) - if version == LLAMA_STACK_RUN_CONFIG_VERSION: - processed_config_dict = replace_env_vars(config_dict) - return StackRunConfig(**cast_image_name_to_string(processed_config_dict)) - if "routing_table" in config_dict: logger.info("Upgrading config...") config_dict = upgrade_from_routing_table(config_dict) config_dict["version"] = LLAMA_STACK_RUN_CONFIG_VERSION - if not config_dict.get("external_providers_dir", None): - config_dict["external_providers_dir"] = EXTERNAL_PROVIDERS_DIR - processed_config_dict = replace_env_vars(config_dict) return StackRunConfig(**cast_image_name_to_string(processed_config_dict)) diff --git a/tests/unit/cli/test_stack_config.py b/tests/unit/cli/test_stack_config.py index 0977a1e43..5d54c2257 100644 --- a/tests/unit/cli/test_stack_config.py +++ b/tests/unit/cli/test_stack_config.py @@ -206,3 +206,26 @@ def test_parse_and_maybe_upgrade_config_invalid(invalid_config): def test_parse_and_maybe_upgrade_config_image_name_int(config_with_image_name_int): result = parse_and_maybe_upgrade_config(config_with_image_name_int) assert isinstance(result.image_name, str) + + +def test_parse_and_maybe_upgrade_config_sets_external_providers_dir(up_to_date_config): + """Test that external_providers_dir is None when not specified (deprecated field).""" + # Ensure the config doesn't have external_providers_dir set + assert "external_providers_dir" not in up_to_date_config + + result = parse_and_maybe_upgrade_config(up_to_date_config) + + # Verify external_providers_dir is None (not set to default) + # This aligns with the deprecation of external_providers_dir + assert result.external_providers_dir is None + + +def test_parse_and_maybe_upgrade_config_preserves_custom_external_providers_dir(up_to_date_config): + """Test that custom external_providers_dir values are preserved.""" + custom_dir = "/custom/providers/dir" + up_to_date_config["external_providers_dir"] = custom_dir + + result = parse_and_maybe_upgrade_config(up_to_date_config) + + # Verify the custom value was preserved + assert str(result.external_providers_dir) == custom_dir