From b3d86ca9261dd87f5b2b460622df49ce7016d7de Mon Sep 17 00:00:00 2001 From: Nathan Weinberg <31703736+nathan-weinberg@users.noreply.github.com> Date: Tue, 15 Jul 2025 12:44:21 -0400 Subject: [PATCH] fix: stop image_name from being cast to an integer (#2759) # What does this PR do? https://github.com/meta-llama/llama-stack/pull/2490 introduced a new function for type conversion of strings. However, a side effect of this is that it will cast any string that can be cast to an integer if possible, which for something like `image_name` is not desired as we only accept strings for this field in the `StackRunConfig` This PR introduces logic to ensure that `image_name` remains a string Closes #2749 ## Test Plan You can run the original step to reproduce from the bug to verify this manually ```bash OPENAI_API_KEY=bogus llama stack build --image-type venv --image-name 2745 --providers inference=remote::openai --run ``` I have also added an additional unit test to prevent any future regression here Signed-off-by: Nathan Weinberg --- llama_stack/distribution/configure.py | 8 +++-- llama_stack/distribution/server/server.py | 3 +- llama_stack/distribution/stack.py | 7 +++++ tests/unit/cli/test_stack_config.py | 36 +++++++++++++++++++++++ 4 files changed, 50 insertions(+), 4 deletions(-) diff --git a/llama_stack/distribution/configure.py b/llama_stack/distribution/configure.py index 35b216b30..2238eef93 100644 --- a/llama_stack/distribution/configure.py +++ b/llama_stack/distribution/configure.py @@ -17,7 +17,7 @@ from llama_stack.distribution.distribution import ( builtin_automatically_routed_apis, get_provider_registry, ) -from llama_stack.distribution.stack import replace_env_vars +from llama_stack.distribution.stack import cast_image_name_to_string, replace_env_vars from llama_stack.distribution.utils.config_dirs import EXTERNAL_PROVIDERS_DIR from llama_stack.distribution.utils.dynamic import instantiate_class_type from llama_stack.distribution.utils.prompt_for_config import prompt_for_config @@ -164,7 +164,8 @@ 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: - return StackRunConfig(**replace_env_vars(config_dict)) + 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...") @@ -175,4 +176,5 @@ def parse_and_maybe_upgrade_config(config_dict: dict[str, Any]) -> StackRunConfi if not config_dict.get("external_providers_dir", None): config_dict["external_providers_dir"] = EXTERNAL_PROVIDERS_DIR - return StackRunConfig(**replace_env_vars(config_dict)) + processed_config_dict = replace_env_vars(config_dict) + return StackRunConfig(**cast_image_name_to_string(processed_config_dict)) diff --git a/llama_stack/distribution/server/server.py b/llama_stack/distribution/server/server.py index f2e29a6f9..974064b58 100644 --- a/llama_stack/distribution/server/server.py +++ b/llama_stack/distribution/server/server.py @@ -47,6 +47,7 @@ from llama_stack.distribution.server.routes import ( initialize_route_impls, ) from llama_stack.distribution.stack import ( + cast_image_name_to_string, construct_stack, replace_env_vars, validate_env_pair, @@ -439,7 +440,7 @@ def main(args: argparse.Namespace | None = None): logger.error(f"Error: {str(e)}") sys.exit(1) config = replace_env_vars(config_contents) - config = StackRunConfig(**config) + config = StackRunConfig(**cast_image_name_to_string(config)) # now that the logger is initialized, print the line about which type of config we are using. logger.info(log_line) diff --git a/llama_stack/distribution/stack.py b/llama_stack/distribution/stack.py index 8d7307b03..98634d8c9 100644 --- a/llama_stack/distribution/stack.py +++ b/llama_stack/distribution/stack.py @@ -267,6 +267,13 @@ def _convert_string_to_proper_type(value: str) -> Any: return value +def cast_image_name_to_string(config_dict: dict[str, Any]) -> dict[str, Any]: + """Ensure that any value for a key 'image_name' in a config_dict is a string""" + if "image_name" in config_dict and config_dict["image_name"] is not None: + config_dict["image_name"] = str(config_dict["image_name"]) + return config_dict + + def validate_env_pair(env_pair: str) -> tuple[str, str]: """Validate and split an environment variable key-value pair.""" try: diff --git a/tests/unit/cli/test_stack_config.py b/tests/unit/cli/test_stack_config.py index d2b6f4b08..a41049006 100644 --- a/tests/unit/cli/test_stack_config.py +++ b/tests/unit/cli/test_stack_config.py @@ -15,6 +15,37 @@ from llama_stack.distribution.configure import ( ) +@pytest.fixture +def config_with_image_name_int(): + return yaml.safe_load( + f""" + version: {LLAMA_STACK_RUN_CONFIG_VERSION} + image_name: 1234 + apis_to_serve: [] + built_at: {datetime.now().isoformat()} + providers: + inference: + - provider_id: provider1 + provider_type: inline::meta-reference + config: {{}} + safety: + - provider_id: provider1 + provider_type: inline::meta-reference + config: + llama_guard_shield: + model: Llama-Guard-3-1B + excluded_categories: [] + disable_input_check: false + disable_output_check: false + enable_prompt_guard: false + memory: + - provider_id: provider1 + provider_type: inline::meta-reference + config: {{}} + """ + ) + + @pytest.fixture def up_to_date_config(): return yaml.safe_load( @@ -125,3 +156,8 @@ def test_parse_and_maybe_upgrade_config_old_format(old_config): def test_parse_and_maybe_upgrade_config_invalid(invalid_config): with pytest.raises(KeyError): parse_and_maybe_upgrade_config(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)