Upgrade Rust to 1.91 and update dependencies#10
Conversation
WalkthroughThe PR updates project dependencies (tokio, serde, chrono, uuid, and others), bumps versions to 0.2.0 (Cargo.toml) and 0.3.0 (VERSION file), adds Rust toolchain configuration specifying channel 1.91, and standardizes code formatting through import reordering and whitespace cleanup across the codebase. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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 |
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/infrastructure/filesystem/notify.rs (1)
155-164: Compilation breaks:u64has nois_multiple_ofon stable 1.91On Rust 1.91 the primitive integer types don’t expose
is_multiple_of, so this line fails to compile. Please stick with the modulo check (or import a trait that actually exists in the codebase).- if metrics_enabled && events_forwarded.is_multiple_of(1000) { + if metrics_enabled && events_forwarded % 1000 == 0 {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (32)
.idea/.gitignore(0 hunks)Cargo.toml(2 hunks)Dockerfile(1 hunks)VERSION(1 hunks)rust-toolchain.toml(1 hunks)src/config/mod.rs(1 hunks)src/config/settings.rs(10 hunks)src/domain/event.rs(1 hunks)src/domain/file.rs(0 hunks)src/domain/state.rs(1 hunks)src/infrastructure/elasticsearch/circuit_breaker.rs(2 hunks)src/infrastructure/elasticsearch/dead_letter_queue.rs(1 hunks)src/infrastructure/elasticsearch/mod.rs(1 hunks)src/infrastructure/elasticsearch/pool.rs(4 hunks)src/infrastructure/filesystem/metadata_cache.rs(1 hunks)src/infrastructure/filesystem/mod.rs(0 hunks)src/infrastructure/filesystem/notify.rs(2 hunks)src/infrastructure/metrics/mod.rs(1 hunks)src/infrastructure/metrics/server.rs(2 hunks)src/infrastructure/mod.rs(1 hunks)src/integration_tests.rs(3 hunks)src/main.rs(1 hunks)src/retry.rs(1 hunks)src/sender.rs(5 hunks)src/task_pool.rs(1 hunks)src/traits.rs(2 hunks)src/transport/bridge/event_bridge.rs(3 hunks)src/transport/bridge/mod.rs(1 hunks)src/transport/channels/bounded.rs(1 hunks)src/transport/channels/mod.rs(1 hunks)src/transport/mod.rs(0 hunks)src/watcher.rs(2 hunks)
💤 Files with no reviewable changes (4)
- src/domain/file.rs
- .idea/.gitignore
- src/infrastructure/filesystem/mod.rs
- src/transport/mod.rs
🧰 Additional context used
🧬 Code graph analysis (10)
src/infrastructure/elasticsearch/pool.rs (1)
src/infrastructure/elasticsearch/circuit_breaker.rs (1)
create_es_circuit_breaker(260-276)
src/infrastructure/metrics/mod.rs (1)
src/infrastructure/metrics/collector.rs (3)
are_metrics_enabled(214-216)init_metrics(200-206)metrics(209-211)
src/infrastructure/elasticsearch/circuit_breaker.rs (1)
src/traits.rs (1)
health_status(16-16)
src/sender.rs (2)
src/infrastructure/metrics/collector.rs (2)
metrics(209-211)are_metrics_enabled(214-216)src/transport/channels/bounded.rs (1)
sender(93-103)
src/integration_tests.rs (2)
src/transport/channels/bounded.rs (1)
create_bounded_channel(280-303)src/watcher.rs (2)
create_bounded_channel(1410-1410)create_bounded_channel(1504-1504)
src/transport/channels/mod.rs (2)
src/transport/channels/bounded.rs (1)
create_bounded_channel(280-303)src/watcher.rs (2)
create_bounded_channel(1410-1410)create_bounded_channel(1504-1504)
src/watcher.rs (1)
src/infrastructure/metrics/collector.rs (2)
metrics(209-211)are_metrics_enabled(214-216)
src/infrastructure/mod.rs (1)
src/infrastructure/metrics/collector.rs (1)
metrics(209-211)
src/transport/bridge/event_bridge.rs (3)
src/infrastructure/metrics/collector.rs (1)
metrics(209-211)src/transport/channels/bounded.rs (1)
create_bounded_channel(280-303)src/watcher.rs (2)
create_bounded_channel(1410-1410)create_bounded_channel(1504-1504)
src/config/settings.rs (3)
src/infrastructure/elasticsearch/pool.rs (2)
new(317-355)new(458-492)src/transport/channels/bounded.rs (1)
new(17-68)src/watcher.rs (1)
new(57-130)
⏰ 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
🔇 Additional comments (20)
VERSION (1)
1-1: Version bump looks good.The version increment from 0.2.0 to 0.3.0 aligns with the PR objectives of upgrading Rust to 1.91 and updating dependencies.
Dockerfile (1)
1-1: Rust version upgrade looks good.The update from rust:1.89-alpine to rust:1.91-alpine aligns with the PR objectives.
src/domain/state.rs (1)
7-7: Formatting changes across the codebase look good.The import reordering and trailing newline additions across multiple files (src/retry.rs, src/infrastructure/elasticsearch/dead_letter_queue.rs, src/transport/channels/bounded.rs, src/traits.rs, src/domain/event.rs, and src/domain/state.rs) are consistent and improve code organization. These changes follow Rust formatting conventions and don't alter any behavior.
src/infrastructure/elasticsearch/pool.rs (4)
10-10: LGTM! Import reordering for consistency.The import statements have been reorganized for better readability, with multi-line formatting for grouped imports from the same module. This is a non-functional formatting improvement.
Also applies to: 14-23
760-763: LGTM! Test formatting improvement.The multi-line formatting of the assertion improves readability without changing test behavior.
798-800: LGTM! Consistent import formatting.Test imports reorganized to match the multi-line format used elsewhere in the file.
841-843: LGTM! Consistent test import formatting.Import formatting matches the style used in other test functions.
src/config/mod.rs (1)
3-3: LGTM! Alphabetical ordering of exports.The public re-exports have been reordered alphabetically (MetricsConfig before Settings), which is a common convention. This doesn't affect the public API, as both items are still exported.
src/watcher.rs (1)
17-17: LGTM! Import reorganization for clarity.The imports have been reorganized with better grouping and multi-line formatting for related modules. This improves readability without changing any behavior.
Also applies to: 23-29
src/task_pool.rs (2)
7-7: LGTM! Import organization improvement.Imports have been repositioned for better logical grouping, with tracing imports after tokio and the transport channel import after domain imports.
Also applies to: 12-12
15-28: LGTM! Documentation formatting improvement.The module documentation has been reformatted with better spacing and line breaks, making it easier to read and understand the purpose and benefits of the SmartTaskPool.
src/transport/bridge/event_bridge.rs (3)
5-5: LGTM! Import formatting consistency.Tracing import moved and channels import formatted for consistency with the codebase style.
Also applies to: 9-9
292-292: LGTM! Test import addition.Added import for
create_bounded_channelin the test module, consistent with test refactoring.
146-146:is_multiple_ofmethod is available and functioning correctly in Rust 1.91.The target Rust version is 1.91, and Rust 1.82.0 was released October 17, 2024. The method has been used successfully in two locations within the codebase without requiring any feature gates or special compilation flags. The code compiles cleanly on the specified toolchain, confirming the method's availability in Rust 1.91.
src/integration_tests.rs (4)
18-20: LGTM! Test import additions.Added imports for
create_bounded_channelandWatcherat the module level to support the integration tests.
317-317: LGTM! Scoped import for test clarity.The
BoundedChannelimport has been moved into the test function scope where it's actually used, improving code organization.
427-427: LGTM! Log message formatting simplification.The warning message has been converted from a multi-line format to a single line, maintaining the same content with improved formatting.
1503-1504: LGTM! Test formatting improvement.The channel creation has been reformatted to multi-line for better readability, consistent with other test code in the file.
src/infrastructure/metrics/mod.rs (1)
4-5: LGTM! Alphabetical ordering of metric exports.The public re-exports from the collector module have been reordered alphabetically (
are_metrics_enabled,init_metrics,metrics), which is a common convention that improves maintainability. The public API remains unchanged.src/transport/channels/mod.rs (1)
5-7: LGTM! Multi-line export formatting.The public re-exports have been reformatted from a single line to multiple lines, improving readability while maintaining the same public API surface.
| FROM rust:1.91-alpine AS builder | ||
|
|
||
| RUN apk add --no-cache musl-dev pkgconfig && \ | ||
| RUN apk add --no-cache musl-dev pkgconfig gcc make && \ |
There was a problem hiding this comment.
🧩 Analysis chain
Verify necessity of gcc and make packages.
The addition of gcc and make to the Alpine package list suggests that updated dependencies may require C compilation. Ensure these packages are actually needed for the build process.
🏁 Script executed:
#!/bin/bash
# Check if any dependencies require C compilation or linking
# by examining Cargo.toml for native dependencies
# Look for common crates that require C toolchain
rg -n "reqwest|openssl|ring|native-tls|rustls-native-certs" Cargo.toml
# Check build.rs files that might require make/gcc
fd -e rs build.rsLength of output: 97
I'll verify more thoroughly by examining the complete Cargo.toml and checking for any other dependencies that might require C compilation.
---
🏁 Script executed:
```shell
#!/bin/bash
# Get full Cargo.toml to see all dependencies
cat Cargo.toml
# Search for other common native-requiring crates
rg -n "tokio|hyper|tonic|protobuf|prost|cc|bindgen|napi|libxml|sqlite" Cargo.toml
# Check if there are any sys crates (usually require C compilation)
rg -n "\-sys" Cargo.toml
Length of output: 1007
Remove unnecessary gcc and make packages from Alpine image.
The dependency on gcc and make is not justified. Analysis of Cargo.toml shows all dependencies are pure Rust:
reqwestexplicitly usesrustls-tls(pure Rust TLS, not native-tls or OpenSSL)- No sys crates, build.rs files, or C-dependent crates present
- All other dependencies (tokio, serde, chrono, etc.) are pure Rust
These packages should be removed from the RUN command to reduce image size and attack surface.
🤖 Prompt for AI Agents
In Dockerfile around line 3, remove the unnecessary packages `gcc` and `make`
from the `RUN apk add --no-cache musl-dev pkgconfig gcc make && \` line; keep
`musl-dev` and `pkgconfig` if still required, update the RUN command
accordingly, then rebuild the image to verify the build succeeds without `gcc`
and `make`.
Summary by CodeRabbit
Chores
Refactor