-
Notifications
You must be signed in to change notification settings - Fork 14
feat: bump llama-stack to 0.2.23 #54
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.
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
feat: bump llama-stack to 0.2.23 #54
Conversation
WalkthroughBumped llama-stack from 0.2.22 → 0.2.23 across pre-commit and distribution files; updated several provider module versions; added/adjusted pip packages in container files; introduced package deduplication/sorting in the build script; relaxed exact pins in Changes
Sequence Diagram(s)sequenceDiagram
participant Dev as Developer / CI
participant BuildPy as distribution/build.py
participant Container as distribution/Containerfile
participant Pip as pip
rect #F0F9E8
Dev->>BuildPy: request dependency list
BuildPy->>BuildPy: collect packages\npackages = sorted(set(packages))
BuildPy-->>Dev: deduped/sorted package list
end
rect #FFF7E6
Dev->>Container: build image
Container->>Pip: pip install (updated packages,\nllama-stack==0.2.23, --no-cache)
Pip-->>Container: install results
Container-->>Dev: image produced (no runtime control changes)
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (4)
🚧 Files skipped from review as they are similar to previous changes (3)
🧰 Additional context used🧠 Learnings (2)📓 Common learnings📚 Learning: 2025-09-15T14:25:54.837ZApplied to files:
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
🔇 Additional comments (4)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
distribution/build.py (1)
91-97: Fix pymilvus replacement logic to prevent duplicate extras.The string replacement logic will match "pymilvus" even within "pymilvus[milvus-lite]", causing duplicate extras like
pymilvus[milvus-lite][milvus-lite]>=2.4.10(visible in distribution/Containerfile line 31). This invalid syntax may cause pip installation failures.Apply this diff to fix the logic:
# Modify pymilvus package to include milvus-lite extra packages = [ - package.replace("pymilvus", "pymilvus[milvus-lite]") - if "pymilvus" in package + package.replace("pymilvus>=", "pymilvus[milvus-lite]>=") + if package.startswith("pymilvus>=") else package for package in packages ]Alternatively, use a more robust approach with regex:
import re # Modify pymilvus package to include milvus-lite extra packages = [ re.sub(r'^pymilvus(?!\[)', 'pymilvus[milvus-lite]', package) if package.startswith("pymilvus") else package for package in packages ]
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
.pre-commit-config.yaml(1 hunks)distribution/Containerfile(2 hunks)distribution/Containerfile.in(1 hunks)distribution/build.py(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: nathan-weinberg
PR: opendatahub-io/llama-stack-distribution#33
File: distribution/Containerfile:17-21
Timestamp: 2025-09-15T14:25:54.837Z
Learning: In the opendatahub-io/llama-stack-distribution repository, the distribution/Containerfile is auto-generated by distribution/build.py based on configuration in build.yaml. When providers are added to build.yaml, the build script automatically regenerates the Containerfile with the required dependencies. Changes to the Containerfile should not be flagged as manual edits if they correspond to legitimate changes in the build configuration.
📚 Learning: 2025-09-15T14:25:54.837Z
Learnt from: nathan-weinberg
PR: opendatahub-io/llama-stack-distribution#33
File: distribution/Containerfile:17-21
Timestamp: 2025-09-15T14:25:54.837Z
Learning: In the opendatahub-io/llama-stack-distribution repository, the distribution/Containerfile is auto-generated by distribution/build.py based on configuration in build.yaml. When providers are added to build.yaml, the build script automatically regenerates the Containerfile with the required dependencies. Changes to the Containerfile should not be flagged as manual edits if they correspond to legitimate changes in the build configuration.
Applied to files:
distribution/Containerfile.indistribution/Containerfile
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Summary
🔇 Additional comments (4)
distribution/build.py (1)
16-16: LGTM! Version bump is consistent with PR objective.The llama-stack version bump to 0.2.23 aligns with the PR's stated goal.
distribution/Containerfile.in (1)
6-6: LGTM! Template version bump is correct.The llama-stack version bump to 0.2.23 in the Containerfile template is consistent with the PR objective.
.pre-commit-config.yaml (1)
59-59: LGTM! Pre-commit dependency version is consistent.The llama-stack version bump to 0.2.23 in the pre-commit hook dependency matches the version in distribution/build.py BASE_REQUIREMENTS.
distribution/Containerfile (1)
49-49: LGTM! Version bump is correct.The llama-stack version bump to 0.2.23 is consistent with the PR objective.
# What does this PR do? <!-- Provide a short summary of what this PR does and why. Link to relevant issues if applicable. --> This PR ensures there is only one pymilvus dependnecy in req.txt. During bumping LLS version in #54, the CI failed upon illegal `pymilvus` dependency in `req.txt`. See CI [run](https://github.com/opendatahub-io/llama-stack-distribution/actions/runs/18126623537/job/51583337085). <!-- If resolving an issue, uncomment and update the line below --> <!-- Closes #[issue-number] --> Closes https://issues.redhat.com/browse/RHAIENG-1312. ## Test Plan <!-- Describe the tests you ran to verify your changes with result summaries. *Provide clear instructions so the plan can be easily re-executed.* --> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Corrected dependency handling to only add the milvus-lite extra to pymilvus when it’s not already specified, preventing duplicate or malformed package specifications. * **Chores** * Refined build-time package transformation for more reliable distribution behavior. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
cab3105 to
a0c8dc5
Compare
|
rebased 👍🏽 |
a0c8dc5 to
9876ddd
Compare
9876ddd to
6ab2324
Compare
5a812fe to
cd441f2
Compare
73a0df0 to
0eaa51a
Compare
|
the failure is due to older version of A new version of trustyai-fms has been released yesterday https://test.pypi.org/project/llama-stack-provider-trustyai-fms/ bumping down and mid stream ... |
|
raised #56 to bump |
0eaa51a to
5fa4aef
Compare
|
rebased #56 👍🏽 |
5fa4aef to
6f68f76
Compare
70e6584 to
1a56a71
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
distribution/Containerfile (1)
1-2: Regenerate this auto-generated file to resolve pre-commit failures.This file is auto-generated by
distribution/build.py(as noted in the header). The pre-commit hook failure indicates the committed version doesn't match what the generator produces. After fixing the generator or configuration inbuild.yaml, regenerate this file to ensure consistency.Based on learnings
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
.pre-commit-config.yaml(1 hunks)distribution/Containerfile(2 hunks)distribution/Containerfile.in(1 hunks)distribution/README.md(1 hunks)distribution/build.py(2 hunks)distribution/build.yaml(1 hunks)distribution/run.yaml(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- distribution/build.yaml
- distribution/build.py
- distribution/README.md
- .pre-commit-config.yaml
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: nathan-weinberg
PR: opendatahub-io/llama-stack-distribution#33
File: distribution/Containerfile:17-21
Timestamp: 2025-09-15T14:25:54.837Z
Learning: In the opendatahub-io/llama-stack-distribution repository, the distribution/Containerfile is auto-generated by distribution/build.py based on configuration in build.yaml. When providers are added to build.yaml, the build script automatically regenerates the Containerfile with the required dependencies. Changes to the Containerfile should not be flagged as manual edits if they correspond to legitimate changes in the build configuration.
📚 Learning: 2025-09-15T14:25:54.837Z
Learnt from: nathan-weinberg
PR: opendatahub-io/llama-stack-distribution#33
File: distribution/Containerfile:17-21
Timestamp: 2025-09-15T14:25:54.837Z
Learning: In the opendatahub-io/llama-stack-distribution repository, the distribution/Containerfile is auto-generated by distribution/build.py based on configuration in build.yaml. When providers are added to build.yaml, the build script automatically regenerates the Containerfile with the required dependencies. Changes to the Containerfile should not be flagged as manual edits if they correspond to legitimate changes in the build configuration.
Applied to files:
distribution/Containerfile.indistribution/Containerfile
🪛 GitHub Actions: Pre-commit
distribution/Containerfile.in
[error] 1-1: Pre-commit hook 'pkg-gen' reported distribution build changes: files were modified by this hook, and the hook ultimately failed. Exit code 1.
distribution/Containerfile
[error] 1-1: Pre-commit hook 'pkg-gen' reported distribution build changes: files were modified by this hook, and the hook ultimately failed. Exit code 1.
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: build-test-push (linux/amd64)
- GitHub Check: Summary
🔇 Additional comments (2)
distribution/run.yaml (1)
107-107: LGTM! Version bump aligns with llama-stack 0.2.23.The llama_stack_provider_lmeval upgrade to 0.3.0 is consistent with the llama-stack 0.2.23 upgrade and reflects the hard dependency noted in the PR comments.
distribution/Containerfile (1)
9-11: Version bumps and dependency consolidation look correct.The changes consolidate dependencies into the initial pip install block and update:
- llama_stack_provider_lmeval: 0.2.4 → 0.3.0
- llama-stack: 0.2.22 → 0.2.23
These changes align with the PR objectives and are consistent with the dependency deduplication logic added to
distribution/build.py. Once the pre-commit hook issue is resolved by regenerating the file, these changes should be correct.Based on learnings
Also applies to: 43-43, 50-50
1a56a71 to
2d20d51
Compare
https://github.com/llamastack/llama-stack/releases/tag/v0.2.23 Relates to: RHAIENG-1311 Signed-off-by: Mustafa Elbehery <[email protected]>
https://pypi.org/project/llama-stack-provider-lmeval/0.3.0/ Relates to: RHAIENG-1380 Signed-off-by: Mustafa Elbehery <[email protected]>
2d20d51 to
c1ca410
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
.pre-commit-config.yaml(1 hunks)distribution/Containerfile(2 hunks)distribution/Containerfile.in(1 hunks)distribution/README.md(2 hunks)distribution/build.py(2 hunks)distribution/build.yaml(1 hunks)distribution/run.yaml(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- distribution/build.py
- distribution/Containerfile.in
- distribution/build.yaml
- distribution/Containerfile
🧰 Additional context used
🪛 GitHub Actions: Pre-commit
distribution/run.yaml
[error] 104-105: pre-commit check-yaml failed: could not find expected ':' while scanning a simple key in distribution/run.yaml (lines 104-105).
[error] 104-105: pre-commit Distribution Documentation hook (doc-gen) failed due to YAML parsing error in distribution/run.yaml (lines 104-105).
🪛 YAMLlint (1.37.1)
distribution/run.yaml
[error] 105-105: syntax error: could not find expected ':'
(syntax)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: build-test-push (linux/amd64)
- GitHub Check: Summary
c1ca410 to
0d4dec1
Compare
|
Checks are passing so other than my comment we are looking good! |
cdoern
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1 to Nathans comment, but lgtm otherwise.
0d4dec1 to
07ee03f
Compare
07ee03f to
e5b62b5
Compare
see trustyai-explainability/community#67 Closes https://issues.redhat.com/browse/RHAIENG-1386 Signed-off-by: Mustafa Elbehery <[email protected]
e5b62b5 to
a21e7f3
Compare
|
LGTM, thanks. |
|
@Elbehery as a reminder, you should be using the bot to merge, not merging things yourself |
What does this PR do?
This PR bumps LLS version to upstream 0.2.23 tag.
https://github.com/llamastack/llama-stack/releases/tag/v0.2.23
Closes: https://issues.redhat.com/browse/RHAIENG-1311
Summary by CodeRabbit
Chores
Documentation
Notes