chore: sync downstream changes to midstream main#13
chore: sync downstream changes to midstream main#13leseb merged 1 commit intoopendatahub-io:mainfrom
main#13Conversation
WalkthroughAdds a pre-commit GitHub Actions workflow and a repository-level pre-commit config. Migrates Red Hat distribution assets from redhat-distribution/ to distribution/, updates README references, bumps llama-stack to 0.2.18, adjusts Containerfile entrypoint to llama_stack.core.server.server, introduces distribution/build.yaml, and updates build script/paths. Removes redhat-distribution/build.yaml. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User as Container Runtime
participant Img as Container Image
participant App as Llama Stack Server
rect rgb(235, 245, 255)
note over Img: Previous flow (removed)
User->>Img: Run image
Img->>App: python -m llama_stack.distribution.server.server --config /opt/app-root/run.yaml
App-->>User: Start server
end
rect rgb(235, 255, 235)
note over Img: New flow
User->>Img: Run image
Img->>App: python -m llama_stack.core.server.server /opt/app-root/run.yaml
App-->>User: Start server
end
sequenceDiagram
autonumber
actor GH as GitHub
participant Runner as ubuntu-latest
participant PreC as pre-commit
participant Git as git
GH->>Runner: Trigger workflow (push to main / PR)
Runner->>Runner: Setup Python 3.11 + cache
Runner->>PreC: Run pre-commit (ruff + hooks)
PreC-->>Runner: Exit with status
Runner->>Git: git diff --exit-code
Git-->>Runner: Clean or dirty
Runner->>Git: git ls-files --others --exclude-standard
Git-->>Runner: Untracked files?
alt Any changes detected
Runner-->>GH: Fail job with instructions
else Clean
Runner-->>GH: Success
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
nathan-weinberg
force-pushed
the
sync-ds-to-main
branch
3 times, most recently
from
59da090 to
560d242
Compare
Signed-off-by: Nathan Weinberg <nweinber@redhat.com>
nathan-weinberg
force-pushed
the
sync-ds-to-main
branch
from
560d242 to
4442b05
Compare
nathan-weinberg
requested review from
Elbehery,
VaishnaviHire,
cdoern,
derekhiggins,
leseb,
rhdedgar and
rhuss
as code owners
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (6)
.github/workflows/redhat-distro-container-build.yml (1)
49-58: Optional: add BuildKit cache and SBOM/provenance to speed builds and improve supply-chain posture.
Small tweak to docker/build-push-action.Apply this diff inside the build-push step:
uses: docker/build-push-action@263435318d21b8e681c14492fe198d362a7d2c83 # v6.18.0 with: context: . file: distribution/Containerfile platforms: ${{ matrix.platform }} push: ${{ github.event_name == 'push' }} tags: ${{ env.IMAGE_NAME }}:${{ github.sha }},${{ env.IMAGE_NAME }}:latest + cache-from: type=gha,scope=llama-stack-distro + cache-to: type=gha,mode=max,scope=llama-stack-distro + provenance: mode=max + sbom: trueREADME.md (1)
38-39: Make example image tag consistent with CI.
CI uses :latest and :${sha}. Consider mirroring that in the example.-podman build --platform linux/amd64 -f distribution/Containerfile -t rh . +podman build --platform linux/amd64 -f distribution/Containerfile -t quay.io/opendatahub/llama-stack:latest .distribution/Containerfile (2)
20-20: Consider pinning or using a constraints/lock for reproducibility.
Most packages are unpinned; a constraints.txt (or --require-hashes) would reduce drift.
45-46: Avoid relying on APP_ROOT env being pre-set in base image.
Use an absolute path to prevent COPY failures if APP_ROOT isn’t defined.-COPY distribution/run.yaml ${APP_ROOT}/run.yaml +COPY distribution/run.yaml /opt/app-root/run.yamldistribution/build.yaml (1)
31-33: Pin additional_pip_packages to versions.
Prevents silent upgrades breaking generated deps.distribution/Containerfile.in (1)
8-9: Use absolute path to avoid template/env coupling.
Same rationale as the generated Containerfile.-COPY distribution/run.yaml ${{APP_ROOT}}/run.yaml +COPY distribution/run.yaml /opt/app-root/run.yaml
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (9)
.github/workflows/pre-commit.yml(1 hunks).github/workflows/redhat-distro-container-build.yml(1 hunks).pre-commit-config.yaml(1 hunks)README.md(3 hunks)distribution/Containerfile(3 hunks)distribution/Containerfile.in(1 hunks)distribution/build.py(5 hunks)distribution/build.yaml(1 hunks)redhat-distribution/build.yaml(0 hunks)
💤 Files with no reviewable changes (1)
- redhat-distribution/build.yaml
🔇 Additional comments (23)
.github/workflows/redhat-distro-container-build.yml (2)
54-54: Path switch to distribution/Containerfile looks good.
Matches the repo-wide migration away from redhat-distribution.
57-58: Verify tag strategy aligns with README and downstream expectations.
Workflow pushes :latest and :${sha}, while README shows a different tag. Align to prevent confusion.README.md (2)
18-19: LGTM on switching generation command to distribution/build.py.
Consistent with the new layout.
28-29: LGTM on template path update.distribution/Containerfile (2)
43-43: Bump to llama-stack==0.2.18 looks correct.
Matches the entrypoint change to core.server.server.
48-48: Confirm positional config arg with 0.2.18.
Ensure llama_stack.core.server.server loads /opt/app-root/run.yaml when passed positionally (no --config).distribution/build.yaml (1)
36-36: external_providers_dir path looks correct.
Matches COPY in the Containerfile.distribution/Containerfile.in (2)
6-6: LGTM on llama-stack==0.2.18.
11-11: Confirm entrypoint arg semantics for 0.2.18.
Validate that passing run.yaml positionally is the intended interface.distribution/build.py (9)
8-8: Path reference updated correctly.The usage comment accurately reflects the new path structure after migrating from
redhat-distribution/todistribution/.
62-62: Path updated correctly for distribution migration.The build configuration path has been appropriately changed from
redhat-distribution/build.yamltodistribution/build.yaml, consistent with the project's migration from the redhat-distribution directory structure.
64-66: Good formatting improvement.The multiline subprocess call formatting improves readability while maintaining the same functionality.
79-81: Enhanced package sorting logic.The improved package sorting with deduplication using
sorted(set(...))is a good enhancement that ensures consistent and clean dependency listings.
111-111: Enhanced error reporting.Adding stderr output to the exception handler provides better debugging information when subprocess calls fail.
117-118: Template and output paths updated correctly.Both the template input path (
distribution/Containerfile.in) and output path (distribution/Containerfile) have been properly updated to reflect the new directory structure.
129-129: Generated file attribution updated.The warning banner correctly references the new script location at
distribution/build.pyinstead of the oldredhat-distribution/build.pypath.
132-134: Formatting consistency maintained.The containerfile content generation logic maintains proper formatting with the
rstrip()call to ensure clean output.
16-16: Confirmed llama-stack==0.2.18 is published on PyPI. The version bump from 0.2.14 to 0.2.18 is valid..github/workflows/pre-commit.yml (1)
1-46: Well-configured pre-commit CI workflow.The workflow follows GitHub Actions best practices with:
- Pinned action versions with commit hashes for security
- Proper concurrency controls to prevent resource conflicts
- Python 3.11 setup with pip caching
- Environment variables for Ruff output formatting
- Post-execution verification to ensure a clean working tree
The verification steps correctly check for both uncommitted changes and new untracked files, providing clear error messages for developers.
.pre-commit-config.yaml (4)
1-5: Standard pre-commit configuration.The global exclude pattern and Python version setting are appropriately configured. Using Python 3.12 as the default language version is current and well-supported.
7-29: Comprehensive pre-commit hooks configuration.The selection of hooks from
pre-commit-hooksis well-balanced, covering essential code quality checks:
- Merge conflict detection with
--assume-in-mergefor Git workflow safety- File size limits (1MB) to prevent accidental large file commits
- Branch protection via
no-commit-to-branch- YAML safety checks with
--unsafefor flexibility- Security via private key detection
- Formatting consistency for line endings and file structure
The exclusion of Python files from trailing-whitespace is sensible since Ruff handles Python formatting.
31-36: Modern Ruff integration.Using Ruff v0.9.4 with both linting (
ruff --fix) and formatting (ruff-format) provides comprehensive Python code quality enforcement. The auto-fix feature helps maintain consistent code style automatically.
40-49: Distribution build integration with version consistency.The local "Distribution Build" hook correctly targets
distribution/build.pyand uses the same llama-stack version (0.2.18) that was updated in the build script. This ensures consistency between the pre-commit environment and the actual build process.The hook configuration is appropriate:
files: ^distribution/.*$targets the right directoryalways_run: trueensures it runs even without distribution file changespass_filenames: falsesince the script doesn't need individual filenamesrequire_serial: trueprevents parallel execution conflicts
cdoern
left a comment
cdoern
left a comment
There was a problem hiding this comment.
lgtm at first glance. thanks for changing the directory names.
5 participants
Summary by CodeRabbit