llama-stack
llama-stack copied to clipboard
feat: remove usage of build yaml for list-deps
What does this PR do?
the build.yaml is only used in the following ways:
- list-deps
- distribution code-gen
since llama stack build no longer exists, I found myself asking "why do we need two different files for list-deps and run"?
Removing the BuildConfig and altering the usage of the DistributionTemplate in llama stack list-deps is the first step in removing the build yaml entirely.
Removing the BuildConfig and build.yaml cuts the files users need to maintain in half, and allows us to focus on the stability of just the run.yaml
The build.yaml made sense for when we were managing the build process for the user and actually producing a run.yaml from the build.yaml, but now that we are simply just getting the provider registry and listing the deps, switching to run.yaml simplifies the scope here greatly.
Test Plan
existing list-deps usage should work in the tests.
hmmm, need to fix CI. seems the env variables are causing issues.
Is there a corresponding change to documentation?
@raghotham , I believe the current documentation only shows llama stack list-deps DISTRO_NAME rather than using a specific yaml config. I will do a sweep of the docs to make sure though!
Removing the BuildConfig and DistributionTemplate
why do we want to remove DistributionTemplate? that is fundamentally how we specify what a distro contains.
@ashwinb you're right, that was an oversight on my part. instead DistributionTemplate would probably change a bit down the line to use the same datatypes as the Providers do in the StackRunConfig. I noticed things like BuildProvider and such in DistributionTemplate.
@leseb , I was thinking the actual removal of BuildConfig and essentially build.yaml could happen in a follow up? I could also do it here, but that is why build.yaml is still mentioned in places, we still generate them.
I kind of think the scope of this is expanding, and doing a follow up for the final "kill" of build.yaml might make sense
This pull request has merge conflicts that must be resolved before it can be merged. @cdoern please rebase it. https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/working-with-forks/syncing-a-fork
I seem to have 2 seperate problems trying this out
The first when ~/.llama/distributions doesn't exist
(.venv) (base) derekh@laptop:~/workarea/llama-stack$ uv run llama stack list-deps --format uv ci-tests
Traceback (most recent call last):
File "/home/derekh/workarea/llama-stack/.venv/bin/llama", line 10, in <module>
sys.exit(main())
^^^^^^
File "/home/derekh/workarea/llama-stack/src/llama_stack/cli/llama.py", line 52, in main
parser.run(args)
File "/home/derekh/workarea/llama-stack/src/llama_stack/cli/llama.py", line 43, in run
args.func(args)
File "/home/derekh/workarea/llama-stack/src/llama_stack/cli/stack/list_deps.py", line 55, in _run_stack_list_deps_command
return run_stack_list_deps_command(args)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/home/derekh/workarea/llama-stack/src/llama_stack/cli/stack/_list_deps.py", line 69, in run_stack_list_deps_command
config_file = resolve_config_or_distro(args.config, Mode.RUN)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/home/derekh/workarea/llama-stack/src/llama_stack/core/utils/config_resolution.py", line 77, in resolve_config_or_distro
raise ValueError(_format_resolution_error(config_or_distro, mode))
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/home/derekh/workarea/llama-stack/src/llama_stack/core/utils/config_resolution.py", line 95, in _format_resolution_error
available_distros = _get_available_distros()
^^^^^^^^^^^^^^^^^^^^^^^^
File "/home/derekh/workarea/llama-stack/src/llama_stack/core/utils/config_resolution.py", line 120, in _get_available_distros
+ [d.name for d in DISTRIBS_BASE_DIR.iterdir() if d.is_dir() and not d.name.startswith(".")]
^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/usr/lib64/python3.12/pathlib.py", line 1056, in iterdir
for name in os.listdir(self):
^^^^^^^^^^^^^^^^
FileNotFoundError: [Errno 2] No such file or directory: '/home/derekh/.llama/distributions'
(.venv) (base) derekh@laptop:~/workarea/llama-stack$
then when I create the dir I get a different error
venv) (base) derekh@laptop:~/workarea/llama-stack$ uv run llama stack list-deps --format uv ci-tests
Could not parse config file ci-tests: Could not resolve config or distribution 'ci-tests'.
Tried the following locations:
1. As file path: /home/derekh/workarea/llama-stack/ci-tests
2. As distribution: /home/derekh/workarea/llama-stack/src/llama_stack/distributions/ci-tests/run.yaml
3. As built distribution: (/home/derekh/.llama/distributions/llamastack-ci-tests/ci-tests-run.yaml, /home/derekh/.llama/distributions/ci-tests/ci-tests-run.yaml)
Available distributions: starter-gpu, postgres-demo, dell, watsonx, oci, ci-tests, starter, meta-reference-gpu, nvidia, __pycache__, open-benchmark
Did you mean one of these distributions?
- ci-tests
- oci
(.venv) (base) derekh@laptop:~/workarea/llama-stack$
@leseb , I was thinking the actual removal of
BuildConfigand essentiallybuild.yamlcould happen in a follow up? I could also do it here, but that is whybuild.yamlis still mentioned in places, we still generate them.I kind of think the scope of this is expanding, and doing a follow up for the final "kill" of build.yaml might make sense
The way I'm seeing this is that list-deps is a dependency of Build today. Our goal is to remove Build, once we do, the list-deps reliance will be removed too. I don't mind the followup but I'd prefer to do the removal in a single PR, it's easier to determine which PR removed the support. Instead of having various PRs removing small portions of it.
What do you think?
@leseb
it's easier to determine which PR removed the support. Instead of having various PRs removing small portions of it.
that makes sense, I will just do the full removal in this PR, let me add some commits.
This pull request has merge conflicts that must be resolved before it can be merged. @cdoern please rebase it. https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/working-with-forks/syncing-a-fork
@ashwinb re-requesting your review on this since I changed a bunch since you approved per @leseb 's comments to remove the build.yaml in its entirety in this PR.
backwards compatibility will fail on this since run.yaml has been renamed config.yaml
This pull request has merge conflicts that must be resolved before it can be merged. @cdoern please rebase it. https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/working-with-forks/syncing-a-fork
config.yamlseems like a really generic name and conveys nothing useful about its content. It did make some sense to have build-config and run-config.Is it possible to come up with a more useful name - which indicates that it has providers (and connector) details?
Hum, this is "just" a configuration file for the llama-stack app. People can choose to name it differently. I don't think we need to find a different name that conveys its content. config.yaml is actually consistent with how most applications name their config. The most important thing to convey is that it is the configuration file for our app, which is what config in this case :)
I agree with @leseb , I don't see a more appropriate name here. The only things I can think of are providers_config and stack_config, but these seem unnecessary to me.