fix: build on midstream main #115
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -11,9 +11,10 @@ | |
| import subprocess | ||
| import sys | ||
| import os | ||
| import shlex | ||
| from pathlib import Path | ||
|
|
||
| CURRENT_LLAMA_STACK_VERSION = "main" | ||
| LLAMA_STACK_VERSION = os.getenv("LLAMA_STACK_VERSION", CURRENT_LLAMA_STACK_VERSION) | ||
|
Comment on lines
+17
to
18
Contributor
There was a problem hiding this comment. 🧩 Analysis chainVerify version change from pre-release to "main" branch.
Consider:
🌐 Web query: 💡 Result: Short answer: pin immutable refs (tags or commit hashes) for production / reproducible builds; use branches only for tracking live development or when you deliberately want moving targets — but validate them with CI and lockfiles. Key practices:
Selected sources:
If you want, I can: (a) show concrete manifest examples for pip/Poetry/Pipenv, or (b) draft a short CI job that enforces and tests git-pinned deps. I'll investigate the codebase context to understand the implications of this version change and whether there are existing mitigations (lockfiles, CI validation, etc.). 🏁 Script executed: #!/bin/bash
# Investigate build.py and version usage patterns
# 1. Check build.py context around lines 17-18
echo "=== build.py context (lines 10-25) ==="
head -30 distribution/build.py | tail -20
# 2. Check for lockfiles
echo -e "\n=== Looking for lockfiles ==="
find . -maxdepth 2 -type f \( -name "*.lock" -o -name "Pipfile*" -o -name "poetry.lock" \) 2>/dev/null | head -10
# 3. Check for CI configuration
echo -e "\n=== CI configuration files ==="
find . -maxdepth 2 -type f \( -name ".github" -o -name "*.yml" -o -name "*.yaml" \) 2>/dev/null | grep -E "(workflows|\.github)" | head -10
# 4. Search for LLAMA_STACK_VERSION usage
echo -e "\n=== LLAMA_STACK_VERSION usage ==="
grep -r "LLAMA_STACK_VERSION" --include="*.py" 2>/dev/null | head -15
# 5. Check README or docs for version strategy
echo -e "\n=== README mention of versions ==="
grep -i "version\|main\|branch" README.md 2>/dev/null | head -10Length of output: 1761 🏁 Script executed: #!/bin/bash
# Check how LLAMA_STACK_VERSION is actually used in installation logic
# 1. Find and show install_llama_stack_from_source function
echo "=== install_llama_stack_from_source implementation ==="
grep -n -A 20 "def install_llama_stack_from_source" distribution/build.py
# 2. Check is_install_from_source
echo -e "\n=== is_install_from_source implementation ==="
grep -n -A 10 "def is_install_from_source" distribution/build.py
# 3. Look for any git clone or VCS-based installation
echo -e "\n=== Git/VCS installation patterns ==="
grep -n "git\|clone\|vcs\|@git" distribution/build.py | head -20
# 4. Check if there's a full requirements file or setup.py
echo -e "\n=== Requirements files ==="
find distribution -name "requirements*.txt" -o -name "setup.py" -o -name "setup.cfg"
# 5. Show full BASE_REQUIREMENTS and PINNED_DEPENDENCIES
echo -e "\n=== Full BASE_REQUIREMENTS and PINNED_DEPENDENCIES ==="
grep -n -A 50 "BASE_REQUIREMENTS" distribution/build.py | head -60Length of output: 4378 🏁 Script executed: #!/bin/bash
# Check how LLAMA_STACK_VERSION is actually used in installation logic
# 1. Find and show install_llama_stack_from_source function
echo "=== install_llama_stack_from_source implementation ==="
grep -n -A 20 "def install_llama_stack_from_source" distribution/build.py
# 2. Check is_install_from_source
echo -e "\n=== is_install_from_source implementation ==="
grep -n -A 10 "def is_install_from_source" distribution/build.py
# 3. Look for any git clone or VCS-based installation
echo -e "\n=== Git/VCS installation patterns ==="
grep -n "git\|clone\|vcs\|@git" distribution/build.py | head -20
# 4. Check if there's a full requirements file or setup.py
echo -e "\n=== Requirements files ==="
find distribution -name "requirements*.txt" -o -name "setup.py" -o -name "setup.cfg"
# 5. Show full BASE_REQUIREMENTS and PINNED_DEPENDENCIES
echo -e "\n=== Full BASE_REQUIREMENTS and PINNED_DEPENDENCIES ==="
grep -n -A 50 "BASE_REQUIREMENTS" distribution/build.py | head -60Length of output: 4378 Code correctly routes "main" to git-based installation; review concerns remain valid but not critical. The code detects that "main" is not a version string (has no dots) and routes it to git installation: However, the review comment's concerns are valid:
The three original suggestions remain sound:
🤖 Prompt for AI Agents |
||
| BASE_REQUIREMENTS = [ | ||
| f"llama-stack=={LLAMA_STACK_VERSION}", | ||
|
|
@@ -30,7 +31,7 @@ | |
| "'ibm-cos-sdk==2.14.2'", | ||
| ] | ||
|
|
||
| source_install_command = """RUN uv pip install --no-cache --no-deps git+https://github.com/opendatahub-io/llama-stack.git@{llama_stack_version}""" | ||
|
|
||
|
|
||
| def get_llama_stack_install(llama_stack_version): | ||
|
|
@@ -47,10 +48,10 @@ def is_install_from_source(llama_stack_version): | |
| return "." not in llama_stack_version or "+rhai" in llama_stack_version | ||
|
|
||
|
|
||
| def check_package_installed(package_name): | ||
| """Check if llama binary is installed and accessible.""" | ||
| if not shutil.which(package_name): | ||
| print(f"Error: {package_name} not found. Please install it first.") | ||
| sys.exit(1) | ||
|
|
||
|
|
||
|
|
@@ -87,9 +88,32 @@ def check_llama_stack_version(): | |
| print("Continuing without version validation...") | ||
|
|
||
|
|
||
| def install_llama_stack_from_source(llama_stack_version): | ||
| """Install llama-stack from source using git.""" | ||
| print("installing llama-stack from source...") | ||
| try: | ||
| result = subprocess.run( | ||
| f"uv pip install git+https://github.com/opendatahub-io/llama-stack.git@{llama_stack_version}", | ||
| shell=True, | ||
| check=True, | ||
| capture_output=True, | ||
| text=True, | ||
| ) | ||
| # Print stdout if there's any output | ||
| if result.stdout: | ||
| print(result.stdout) | ||
| except subprocess.CalledProcessError as e: | ||
| print(f"Error installing llama-stack: {e}") | ||
| if e.stdout: | ||
| print(f"stdout: {e.stdout}") | ||
| if e.stderr: | ||
| print(f"stderr: {e.stderr}") | ||
| sys.exit(1) | ||
|
|
||
|
|
||
| def get_dependencies(): | ||
| """Execute the llama stack build command and capture dependencies.""" | ||
| cmd = "llama stack list-deps distribution/build.yaml" | ||
|
Contributor
There was a problem hiding this comment. 🧩 Analysis chainVerify the new command syntax works correctly. The command changed from
🏁 Script executed: #!/bin/bash
# Description: Verify the new list-deps command exists and check its output format
# Check if the command is available and what format it returns
llama stack list-deps --help || echo "Error: list-deps command not found"
# Try running the command to see the output format
if [ -f "distribution/build.yaml" ]; then
echo "=== Testing list-deps command output ==="
llama stack list-deps distribution/build.yaml | head -20
fiLength of output: 164 I'll investigate this further by examining the codebase and checking for documentation on the command change. Good news: the I found important documentation. Let me now examine the actual code to understand the context better. 🏁 Script executed: #!/bin/bash
# Find and examine the get_dependencies function in distribution/build.py
rg -n "get_dependencies" --type=py -A 10 -B 5Length of output: 2004 Fix the command syntax: Official documentation shows The current code at line 93 uses
Also update the docstring on line 92 from "Execute the llama stack build command" to reflect the new command being used. |
||
| try: | ||
| result = subprocess.run( | ||
| cmd, shell=True, capture_output=True, text=True, check=True | ||
|
|
@@ -101,56 +125,91 @@ def get_dependencies(): | |
| no_cache = [] | ||
|
|
||
| for line in result.stdout.splitlines(): | ||
| line = line.strip() | ||
| if not line: # Skip empty lines | ||
| continue | ||
|
|
||
| # Handle both "uv pip" format and direct package list format | ||
| if line.startswith("uv pip"): | ||
| # Legacy format: "uv pip install ..." | ||
| line = line.replace("uv ", "RUN ", 1) | ||
| parts = line.split(" ", 3) | ||
| if len(parts) >= 4: # We have packages to sort | ||
| cmd_parts = parts[:3] # "RUN pip install" | ||
| packages_str = parts[3] | ||
| else: | ||
| standard_deps.append(" ".join(parts)) | ||
| continue | ||
| else: | ||
| # New format: just packages, possibly with flags | ||
| cmd_parts = ["RUN", "uv", "pip", "install"] | ||
| packages_str = line | ||
|
|
||
| # Parse packages and flags from the line | ||
| # Use shlex.split to properly handle quoted package names | ||
| parts_list = shlex.split(packages_str) | ||
| packages = [] | ||
| flags = [] | ||
| extra_index_url = None | ||
|
|
||
| i = 0 | ||
| while i < len(parts_list): | ||
| if parts_list[i] == "--extra-index-url" and i + 1 < len(parts_list): | ||
| extra_index_url = parts_list[i + 1] | ||
| flags.extend([parts_list[i], parts_list[i + 1]]) | ||
| i += 2 | ||
| elif parts_list[i] == "--index-url" and i + 1 < len(parts_list): | ||
| flags.extend([parts_list[i], parts_list[i + 1]]) | ||
| i += 2 | ||
| elif parts_list[i] in ["--no-deps", "--no-cache"]: | ||
| flags.append(parts_list[i]) | ||
| i += 1 | ||
| else: | ||
| packages.append(parts_list[i]) | ||
| i += 1 | ||
|
|
||
| # Sort and deduplicate packages | ||
| packages = sorted(set(packages)) | ||
|
|
||
| # Add quotes to packages with > or < to prevent bash redirection | ||
| packages = [ | ||
| f"'{package}'" if (">" in package or "<" in package) else package | ||
| for package in packages | ||
| ] | ||
|
|
||
| # Modify pymilvus package to include milvus-lite extra | ||
| packages = [ | ||
| package.replace("pymilvus", "pymilvus[milvus-lite]") | ||
| if "pymilvus" in package and "[milvus-lite]" not in package | ||
| else package | ||
| for package in packages | ||
| ] | ||
| packages = sorted(set(packages)) | ||
|
Comment on lines
+180
to
+187
Contributor
There was a problem hiding this comment. 🧩 Analysis chainVerify and document the pymilvus package modification. The code automatically replaces
Additionally, note that line 164 sorts the packages again after they were already sorted on line 149, which is redundant. Consider:
🌐 Web query: 💡 Result: Short answer: Yes — the Milvus docs recommend installing pymilvus (with the milvus‑lite extra or the bundled milvus‑lite) for a local/single‑file "Milvus Lite" experience. Use either pip install -U pymilvus (recent pymilvus releases include milvus‑lite) or pip install -U "pymilvus[milvus-lite]" to explicitly request the extra. [1][2][3] What changes vs. the base package
Recommendation: For local development/prototyping, install either pip install -U pymilvus (if using a recent release that bundles milvus‑lite) or pip install -U "pymilvus[milvus-lite]" to be explicit. For production or large‑scale deployments use a standalone/distributed Milvus server instead. [1][2][3] Sources: I have the web search results clarifying that Length of output: 233 Length of output: 1237 Length of output: 4430 Length of output: 86 Remove redundant sort on line 164 and document the pymilvus modification. The
🤖 Prompt for AI Agents |
||
|
|
||
| # Build the command based on flags | ||
| if extra_index_url or "--index-url" in flags: | ||
| # Torch dependencies with extra index URL | ||
| full_cmd = " ".join(cmd_parts + flags + packages) | ||
| torch_deps.append(full_cmd) | ||
| elif "--no-deps" in flags: | ||
| full_cmd = " ".join(cmd_parts + flags + packages) | ||
| no_deps.append(full_cmd) | ||
| elif "--no-cache" in flags: | ||
| full_cmd = " ".join(cmd_parts + flags + packages) | ||
| no_cache.append(full_cmd) | ||
| else: | ||
| # Standard dependencies with multi-line formatting | ||
| formatted_packages = " \\\n ".join(packages) | ||
| full_cmd = f"{' '.join(cmd_parts)} \\\n {formatted_packages}" | ||
| standard_deps.append(full_cmd) | ||
coderabbitai[bot] marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
||
| # Combine all dependencies in specific order | ||
| all_deps = [] | ||
|
|
||
| # Add pinned dependencies FIRST to ensure version compatibility | ||
| if PINNED_DEPENDENCIES: | ||
| pinned_packages = " \\\n ".join(PINNED_DEPENDENCIES) | ||
| pinned_cmd = f"RUN uv pip install --upgrade \\\n {pinned_packages}" | ||
| all_deps.append(pinned_cmd) | ||
|
|
||
| all_deps.extend(sorted(standard_deps)) # Regular pip installs | ||
|
|
@@ -203,8 +262,11 @@ def generate_containerfile(dependencies, llama_stack_install): | |
|
|
||
|
|
||
| def main(): | ||
| check_package_installed("uv") | ||
| install_llama_stack_from_source(LLAMA_STACK_VERSION) | ||
|
|
||
| print("Checking llama installation...") | ||
| check_package_installed("llama") | ||
|
|
||
| # Do not perform version check if installing from source | ||
| if not is_install_from_source(LLAMA_STACK_VERSION): | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.