Skip to content

chore(kv-router): remove native kv-indexer binary, use maturin-built one [DYN-2459]#7338

Merged
PeaBrane merged 15 commits intomainfrom
rupei/nuke-native-kv-indexer-binary
Mar 15, 2026
Merged

chore(kv-router): remove native kv-indexer binary, use maturin-built one [DYN-2459]#7338
PeaBrane merged 15 commits intomainfrom
rupei/nuke-native-kv-indexer-binary

Conversation

@PeaBrane
Copy link
Contributor

@PeaBrane PeaBrane commented Mar 13, 2026

Remove the duplicate dynamo-kv-indexer binary from lib/kv-router and consolidate on the maturin-built binary in lib/bindings/python. Add test-endpoints feature to the Python bindings Cargo.toml so e2e tests can use the pre-built binary instead of cargo run.

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Documentation

    • Updated standalone indexer build instructions with new recommended command sequence and environment setup
  • Refactoring

    • Reorganized standalone indexer build and deployment process for improved Python environment integration
  • Features

    • Added test-endpoints configuration option for specialized indexer builds

Signed-off-by: PeaBrane <yanrpei@gmail.com>
@PeaBrane PeaBrane requested review from a team as code owners March 13, 2026 04:38
@github-actions github-actions bot added chore documentation Improvements or additions to documentation labels Mar 13, 2026
@github-actions
Copy link
Contributor

github-actions bot commented Mar 13, 2026

@PeaBrane PeaBrane enabled auto-merge (squash) March 13, 2026 04:39
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Mar 13, 2026

Walkthrough

Refactors the standalone KV indexer build process from a Rust cargo binary to a Python binding built via maturin. Removes the CLI binary target from the kv-router crate, renames the indexer-bin feature to standalone-indexer, and updates documentation and tests to reflect the new build approach.

Changes

Cohort / File(s) Summary
Build system updates
lib/bindings/python/Cargo.toml, lib/kv-router/Cargo.toml
Added test-endpoints feature to Python bindings. Renamed indexer-bin feature to standalone-indexer in kv-router; updated test-endpoints feature dependency; removed tracing-subscriber optional dependency and the [[bin]] target for dynamo-kv-indexer.
Documentation
docs/components/router/standalone-indexer.md
Updated build instructions from cargo-based binary build to maturin-based Python bindings build; added virtualenv installation note and test build hint.
Removed CLI implementation
lib/kv-router/src/bin/kv_indexer/main.rs
Deleted entire standalone indexer CLI binary including Clap-based Cli struct and async main entry point with argument parsing and server initialization logic.
Test code updates
tests/router/test_router_e2e_with_mockers.py
Removed dynamo-kv-router binary invocation from standalone indexer startup commands in both enter and launch_indexer methods.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Poem

🐰 The indexer hops to Python's way,
No Rust CLI to build today,
Maturin binds what once was free,
A hop, a skip, from src to PyPI!

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Description check ⚠️ Warning The description is incomplete and does not follow the provided template structure with required sections (Overview, Details, Where should the reviewer start, Related Issues). Restructure the description to match the template: add a clear Overview section, expand Details with specific file changes, specify where reviewers should start, and clarify any related GitHub issues.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely summarizes the main change: removing the native kv-indexer binary from lib/kv-router and consolidating on the maturin-built version in lib/bindings/python.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

📝 Coding Plan
  • Generate coding plan for human review comments

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.

Tip

You can make CodeRabbit's review stricter and more nitpicky using the `assertive` profile, if that's what you prefer.

Change the reviews.profile setting to assertive to make CodeRabbit's nitpick more issues in your PRs.

Copy link
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.

🧹 Nitpick comments (1)
docs/components/router/standalone-indexer.md (1)

84-85: Consider adding a copy-paste test-build command.

Small doc polish: include an explicit command using --features test-endpoints to reduce ambiguity for test setup.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@docs/components/router/standalone-indexer.md` around lines 84 - 85, Add a
concrete copy-paste command showing how to install dynamo-kv-indexer into the
virtualenv for test builds that need pause/resume listener endpoints: include
the package name `dynamo-kv-indexer` and the `--features test-endpoints` flag in
the example so users can run it directly (e.g., show the full
pip/poetry/installation command variant you use in this repo). Place the command
immediately after the existing sentence mentioning `dynamo-kv-indexer` and
`--features test-endpoints` so readers have an explicit, ready-to-run example.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@docs/components/router/standalone-indexer.md`:
- Around line 84-85: Add a concrete copy-paste command showing how to install
dynamo-kv-indexer into the virtualenv for test builds that need pause/resume
listener endpoints: include the package name `dynamo-kv-indexer` and the
`--features test-endpoints` flag in the example so users can run it directly
(e.g., show the full pip/poetry/installation command variant you use in this
repo). Place the command immediately after the existing sentence mentioning
`dynamo-kv-indexer` and `--features test-endpoints` so readers have an explicit,
ready-to-run example.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 137a1111-5d02-49bb-96a0-260680679b55

📥 Commits

Reviewing files that changed from the base of the PR and between bddaaa2 and a39c26f.

⛔ Files ignored due to path filters (1)
  • Cargo.lock is excluded by !**/*.lock
📒 Files selected for processing (5)
  • docs/components/router/standalone-indexer.md
  • lib/bindings/python/Cargo.toml
  • lib/kv-router/Cargo.toml
  • lib/kv-router/src/bin/kv_indexer/main.rs
  • tests/router/test_router_e2e_with_mockers.py
💤 Files with no reviewable changes (2)
  • tests/router/test_router_e2e_with_mockers.py
  • lib/kv-router/src/bin/kv_indexer/main.rs

Signed-off-by: PeaBrane <yanrpei@gmail.com>
…er routes

Signed-off-by: PeaBrane <yanrpei@gmail.com>
Signed-off-by: PeaBrane <yanrpei@gmail.com>
Signed-off-by: PeaBrane <yanrpei@gmail.com>
Signed-off-by: PeaBrane <yanrpei@gmail.com>
Signed-off-by: PeaBrane <yanrpei@gmail.com>
Signed-off-by: PeaBrane <yanrpei@gmail.com>
Signed-off-by: PeaBrane <yanrpei@gmail.com>
Signed-off-by: PeaBrane <yanrpei@gmail.com>
Signed-off-by: PeaBrane <yanrpei@gmail.com>
Signed-off-by: PeaBrane <yanrpei@gmail.com>
Signed-off-by: PeaBrane <yanrpei@gmail.com>
@PeaBrane PeaBrane merged commit b6596c5 into main Mar 15, 2026
257 of 259 checks passed
@PeaBrane PeaBrane deleted the rupei/nuke-native-kv-indexer-binary branch March 15, 2026 08:11
@PeaBrane PeaBrane changed the title chore(kv-router): remove native kv-indexer binary, use maturin-built one chore(kv-router): remove native kv-indexer binary, use maturin-built one [DYN-2459] Mar 15, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

actions chore container documentation Improvements or additions to documentation size/XXL

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants