Skip to content

fix: bring back milvus-lite#44

Merged
nathan-weinberg merged 1 commit intoopendatahub-io:mainfrom
nathan-weinberg:fix-milvus
Sep 18, 2025
Merged

fix: bring back milvus-lite#44
nathan-weinberg merged 1 commit intoopendatahub-io:mainfrom
nathan-weinberg:fix-milvus

Conversation

@nathan-weinberg
Copy link
Copy Markdown
Collaborator

@nathan-weinberg nathan-weinberg commented Sep 18, 2025

What does this PR do?

milvus no longer includes milvus-lite by default

see: milvus-io/pymilvus#2976

this commit modifies our build script to include it otherwise the inline::milvus provider fails on boot

Test Plan

CI should pass

Summary by CodeRabbit

  • New Features
    • Enabled default support for Milvus Lite via pymilvus[milvus-lite], simplifying local/vector DB usage.
  • Chores
    • Updated container configuration to install pymilvus with the milvus-lite extra.
    • Adjusted build process to automatically rewrite pymilvus dependencies to include milvus-lite.

milvus no longer includes milvus-lite by default
see: milvus-io/pymilvus#2976

this commit modifies our build script to include it
otherwise the inline::milvus provider fails on boot

Signed-off-by: Nathan Weinberg <nweinber@redhat.com>
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Sep 18, 2025

Walkthrough

Updates dependency handling to require the pymilvus milvus-lite extra. The Containerfile switches to installing pymilvus with [milvus-lite] constraint, and build.py rewrites any pymilvus dependency to pymilvus[milvus-lite] during dependency resolution.

Changes

Cohort / File(s) Summary
Milvus lite dependency handling
distribution/Containerfile, distribution/build.py
Containerfile: change requirement from pymilvus>=2.4.10 to pymilvus[milvus-lite]>=2.4.10. build.py: in get_dependencies(), rewrite pymilvus to pymilvus[milvus-lite] within uv pip parsing before assembling install commands.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  participant Dev as build.py
  participant UV as uv pip parser
  participant Pip as pip install

  Dev->>UV: Parse dependencies (requirements/constraints)
  UV-->>Dev: Package list (includes pymilvus)
  note over Dev: Rewrite "pymilvus" -> "pymilvus[milvus-lite]"
  Dev->>Pip: Install commands with rewritten deps
  Pip-->>Dev: Installation result
Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

I nibbled deps with careful bite,
Swapped plain milvus for milvus-lite.
In builds I hop, in containers glide,
Extras packed, neatly applied.
Thump-thump—release feels feather-light! 🐇✨

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The title "fix: bring back milvus-lite" is concise and directly summarizes the PR's primary intent—restoring milvus-lite via pymilvus extras as implemented in distribution/Containerfile and distribution/build.py—so it clearly communicates the main change to reviewers.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
✨ Finishing touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a 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

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between f222f3b and aaac55d.

📒 Files selected for processing (2)
  • distribution/Containerfile (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
⏰ 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: build-test-push (linux/amd64)
🔇 Additional comments (1)
distribution/Containerfile (1)

28-28: Approve: Milvus Lite extra enabled — looks good
Sanity checks passed: exactly one 'pymilvus[milvus-lite]>=2.4.10' in distribution/Containerfile (line 28); no duplicate extras or accidental mutations.

@nathan-weinberg nathan-weinberg merged commit 2b226dd into opendatahub-io:main Sep 18, 2025
5 checks passed
@nathan-weinberg nathan-weinberg deleted the fix-milvus branch September 18, 2025 17:38
@leseb
Copy link
Copy Markdown
Collaborator

leseb commented Sep 19, 2025

Upstream followup llamastack/llama-stack#3488

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants