Skip to content

fix: handle missing external_providers_dir #3974

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Oct 31, 2025
Merged

fix: handle missing external_providers_dir #3974

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion src/llama_stack/cli/stack/run.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,8 @@ def _run_stack_run_cmd(self, args: argparse.Namespace) -> None:

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}")
Expand Down
9 changes: 0 additions & 9 deletions src/llama_stack/core/configure.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@
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
Copy link
Contributor

@raghotham raghotham Oct 29, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do you want to remove the definition of this constant as well?

Copy link
Contributor

@cdoern cdoern Oct 30, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think EXTERNAL_PROVIDERS_DIR is used elsewhere in the codebase

Copy link
Contributor Author

@rhdedgar rhdedgar Oct 30, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi, I can confirm that EXTERNAL_PROVIDERS_DIR is still being used in the generate_run_config function in src/llama_stack/cli/stack/utils.py.

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
Expand Down Expand Up @@ -194,19 +193,11 @@ def get_providers(entries):


def parse_and_maybe_upgrade_config(config_dict: dict[str, Any]) -> StackRunConfig:
version = config_dict.get("version", None)
Copy link
Contributor

@cdoern cdoern Oct 30, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

question here, is this necessary? I might be missing something but this seems like a separate codepath.

Copy link
Contributor Author

@rhdedgar rhdedgar Oct 30, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No worries! This removes code that was used for an early-return micro optimization. Now that it's removed, it will apply the same processing to configs regardless of version.

E.g. configs with version 2 would've skipped the external_providers_dir processing steps that other configs would get.

The removals should align with the intended deprecation of external_providers_dir by treating it as optional here.

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))
23 changes: 23 additions & 0 deletions tests/unit/cli/test_stack_config.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Loading