fix: handle missing external_providers_dir #3974
Conversation
Signed-off-by: Doug Edgar <[email protected]>
rhdedgar
requested review from
ashwinb,
bbrowning,
ehhuang,
franciscojavierarceo,
hardikjshah,
leseb,
mattf,
raghotham,
reluctantfuturist,
slekkala1,
terrytangyuan and
yanxi0830
as code owners
meta-cla
bot
added
the
CLA Signed
| 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 |
There was a problem hiding this comment.
do you want to remove the definition of this constant as well?
There was a problem hiding this comment.
I think EXTERNAL_PROVIDERS_DIR is used elsewhere in the codebase
There was a problem hiding this comment.
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.
|
|
||
|
|
||
| def parse_and_maybe_upgrade_config(config_dict: dict[str, Any]) -> StackRunConfig: | ||
| version = config_dict.get("version", None) |
There was a problem hiding this comment.
question here, is this necessary? I might be missing something but this seems like a separate codepath.
There was a problem hiding this comment.
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.
3 participants
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
modulespecification approach.It addresses the issue in #3950, where using the default provided run.yaml config resulted in the
external_providers_dirparameter being set to the literal stringNone, and crashing the llama-stack server when starting.Closes #3950
Test Plan
podman build . -f containers/Containerfile --build-arg DISTRO_NAME=starter --tag llama-stack:starterpodman run -it localhost/llama-stack:starter