chore: Upgrade Rust to 1.90 #3147
Conversation
Also update the edition on bindings and runtime examples to 2024. I missed that when updating the edition on the main project. Signed-off-by: Graham King <[email protected]>
Signed-off-by: Graham King <[email protected]>
WalkthroughRust toolchain updated from 1.89.0 to 1.90.0 across Earthfile, Dockerfiles, and rust-toolchain files. Rust edition bumped to 2024 in two Cargo.toml files. Minor internal Rust refactors adjust validation, inline a call, rename a variable, and replace modulo checks with is_multiple_of. One Dockerfile adds a trailing newline. Changes
Sequence Diagram(s)Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Poem
Pre-merge checks❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
container/Dockerfile.sglang-wideep (1)
14-14: RUST_VERSION -> 1.90.0 aligned.Consider passing this via build-arg from your Earthfile/CI to avoid future drift rather than hardcoding here.
container/Dockerfile.trtllm_prebuilt (1)
44-44: ENV RUST_VERSION=1.90.0: OK.Minor consistency nit: other Dockerfiles sometimes use ARG for RUST_VERSION; standardizing on ARG+ENV (set ENV from ARG) can simplify overrides.
Apply if you want consistency:
- PATH=/usr/local/cargo/bin:$PATH \ - RUST_VERSION=1.90.0 + PATH=/usr/local/cargo/bin:$PATH +ARG RUST_VERSION=1.90.0 +ENV RUST_VERSION=${RUST_VERSION}container/Dockerfile (1)
96-96: Bump to 1.90.0 and newline: LGTM.Two small follow-ups:
- Optional: source RUST_VERSION from rust-toolchain.toml in CI to prevent drift.
- Security: consider adding a checksum verification for rustup-init (mirrors your TODO in trtllm_prebuilt).
Also applies to: 370-370
lib/bindings/python/rust/engine.rs (1)
183-195: Add early runtime type-check for async generator (verified pyo3-async-runtimes v0.23.0, behind "unstable-streams")If the generator returns a non-async-generator, into_stream_with_locals_v1 will error deeper — fail early with a clear PyTypeError.
- let gen_result = if has_context { + let gen_result = if has_context { // Pass context as a kwarg let kwarg = PyDict::new(py); kwarg.set_item("context", &py_ctx)?; generator.call(py, (py_request,), Some(&kwarg)) } else { // Legacy: No `context` arg generator.call1(py, (py_request,)) }?; + + // Ensure we actually got an async generator (has __anext__) + if !gen_result.bind(py).hasattr("__anext__")? { + return Err(PyErr::new::<pyo3::exceptions::PyTypeError, _>( + "expected an async generator; got a non-generator value", + )); + } let locals = TaskLocals::new(event_loop.bind(py).clone()); - pyo3_async_runtimes::tokio::into_stream_with_locals_v1(locals, gen_result.into_bound(py)) + pyo3_async_runtimes::tokio::into_stream_with_locals_v1(locals, gen_result.into_bound(py))
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
Earthfile(1 hunks)container/Dockerfile(2 hunks)container/Dockerfile.sglang-wideep(1 hunks)container/Dockerfile.trtllm_prebuilt(1 hunks)lib/bindings/python/Cargo.toml(1 hunks)lib/bindings/python/rust/engine.rs(2 hunks)lib/bindings/python/rust/lib.rs(2 hunks)lib/llm/src/mocker/sequence.rs(2 hunks)lib/runtime/examples/Cargo.toml(1 hunks)lib/runtime/examples/rust-toolchain.toml(1 hunks)rust-toolchain.toml(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-08-21T17:23:02.836Z
Learnt from: michaelfeil
PR: ai-dynamo/dynamo#2591
File: lib/bindings/python/rust/http.rs:0-0
Timestamp: 2025-08-21T17:23:02.836Z
Learning: In lib/bindings/python/rust/http.rs, the enable_endpoint method uses EndpointType::all() to dynamically support all available endpoint types with case-insensitive matching, which is more maintainable than hardcoded match statements for endpoint type mappings.
Applied to files:
lib/bindings/python/rust/lib.rs
📚 Learning: 2025-09-11T03:24:47.820Z
Learnt from: kthui
PR: ai-dynamo/dynamo#3004
File: lib/runtime/src/pipeline/network/ingress/push_handler.rs:271-277
Timestamp: 2025-09-11T03:24:47.820Z
Learning: In lib/runtime/src/pipeline/network/ingress/push_handler.rs, the maintainer prefers to keep the existing error comparison logic using format!("{:?}", err) == STREAM_ERR_MSG unchanged until proper error types are implemented, even though it has technical debt. Avoid suggesting changes to working legacy code that will be refactored later.
Applied to files:
lib/bindings/python/rust/lib.rs
🧬 Code graph analysis (1)
lib/llm/src/mocker/sequence.rs (3)
lib/llm/src/block_manager/block/state.rs (1)
tokens(126-132)lib/llm/src/block_manager/block.rs (2)
tokens(315-317)tokens(488-488)lib/llm/src/tokens.rs (4)
tokens(336-338)tokens(434-436)block_size(459-461)block_size(793-795)
⏰ 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). (4)
- GitHub Check: Build and Test - dynamo
- GitHub Check: pre-merge-rust (lib/runtime/examples)
- GitHub Check: pre-merge-rust (.)
- GitHub Check: pre-merge-rust (lib/bindings/python)
🔇 Additional comments (10)
lib/runtime/examples/Cargo.toml (1)
14-14: Workspace examples: edition 2024 aligned.Looks good and consistent with the toolchain bump.
lib/runtime/examples/rust-toolchain.toml (1)
2-2: Toolchain channel 1.90.0: OK.Matches the repo upgrade.
lib/bindings/python/Cargo.toml (1)
11-11: Edition bump to 2024 — LGTM; CI uses Rust 1.90.0.
No "edition = "2021"" found; rust-toolchain.toml (root) and lib/runtime/examples/rust-toolchain.toml set channel = "1.90.0"; edition = "2024" present in lib/bindings/python/Cargo.toml:11, lib/runtime/examples/Cargo.toml:14, Cargo.toml:20. Enable edition-2024 clippy/rustfmt lints in CI.rust-toolchain.toml (1)
2-2: Root toolchain set to 1.90.0 — add drift guardAdd a CI check to fail when Dockerfiles / Earthfile or nested lib/**/rust-toolchain.toml drift from the root rust-toolchain.toml channel. Re-run the verifier below (previous attempts failed due to awk/quoting issues).
Use this script to verify all hardcoded Rust versions match the root rust-toolchain channel:
#!/usr/bin/env bash set -euo pipefail expected="$(sed -n -E 's/^\s*channel\s*=\s*"([^"]+)".*/\1/p' rust-toolchain.toml | head -1)" [ -n "$expected" ] || { echo "Unable to find channel in rust-toolchain.toml" >&2; exit 2; } printf 'Expected Rust: %s\n\n' "$expected" # Use Python for robust parsing (avoids awk/quoting issues) expected="$expected" python3 - <<'PY' import os, re, sys, glob expected = os.environ.get('expected') if not expected: print("expected env var missing", file=sys.stderr); sys.exit(2) semver = re.compile(r'([0-9]+\.[0-9]+\.[0-9]+)') bad = False def check_file(path): try: txt = open(path, 'r', errors='ignore').read() except Exception as e: print(f"Cannot read {path}: {e}", file=sys.stderr) return True # For rust-toolchain.toml files check channel explicitly if path.endswith('rust-toolchain.toml'): m = re.search(r'^\s*channel\s*=\s*"([^"]+)"', txt, re.M) if m: v = m.group(1) if v != expected: print(f"Mismatch: {path} (found {v}, expected {expected})") return True print(f"OK: {path} -> {v}"); return False # Otherwise look for a semver in the file (ARG/ENV RUST_VERSION or FROM rust:1.90.0) m = semver.search(txt) if m: v = m.group(1) if v != expected: print(f"Mismatch: {path} (found {v}, expected {expected})") return True print(f"OK: {path} -> {v}"); return False print(f"No semver found in: {path}"); return True paths = [] paths += glob.glob('container/**/Dockerfile*', recursive=True) paths += glob.glob('container/Dockerfile', recursive=True) paths += glob.glob('**/Earthfile', recursive=True) paths += glob.glob('lib/**/rust-toolchain.toml', recursive=True) if not paths: print("No target files found to check.", file=sys.stderr); sys.exit(2) for p in sorted(set(paths)): bad = check_file(p) or bad sys.exit(1 if bad else 0) PYlib/llm/src/mocker/sequence.rs (2)
31-31: LGTM: Modern Rust idiom adoptionThe use of
is_multiple_ofmethod instead of modulo comparison is cleaner and more expressive. This is a good adoption of Rust 1.90 features.
261-261: LGTM: Consistent modern idiomThis change aligns with the previous update on line 31, maintaining consistency in using the
is_multiple_ofmethod throughout the codebase.lib/bindings/python/rust/lib.rs (2)
738-743: LGTM: Simplified validation logicThe condensed validation logic is more concise while maintaining the same behavior. The
if letpattern combined with&&provides clear early exit when the payload exists but isn't an object.
1089-1094: LGTM: Code optimization by inliningInlining the
pythonizecall within the closure eliminates an unnecessary intermediate variable while maintaining the same functionality and error handling.Earthfile (2)
94-94: LGTM: Rust version upgradeThe environment variable update to Rust 1.90.0 is consistent with the PR objective.
100-100: LGTM: Toolchain consistencyThe rustup-init toolchain version matches the environment variable, ensuring consistency across the build environment.
New edition, new formatting rules. Signed-off-by: Graham King <[email protected]>
Also update the edition on bindings and runtime examples to 2024. I missed that when updating the edition on the main project.
Summary by CodeRabbit
New Features
Refactor
Chores