ci: use stable toolchain#645
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR updates the Rust toolchain to the latest stable version (1.87.0) and aligns various modules to use a boxed error type for AtomaP2pNodeError.
- Bumps Rust default toolchain to 1.87.0 in Dockerfiles and GitHub workflows
- Changes many P2P validation/serialization functions to return
Box<AtomaP2pNodeError>and updates tests accordingly - Refines
println!calls in OpenAPI components and silences a Clippy lint instreamer.rs
Reviewed Changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| atoma-service/src/streamer.rs | Added Clippy suppression for field naming |
| atoma-service/src/components/openapi.rs | Switched println!("{:?}") to {} with display() |
| atoma-p2p/src/utils.rs | Changed validation functions to return boxed errors |
| atoma-p2p/src/types.rs | Updated Result alias and error mapping in serializers |
| atoma-p2p/src/tests.rs | Updated assertions to match boxed error signatures |
| atoma-p2p/src/service.rs | Unboxed errors from deserialize_with_signature |
| atoma-p2p-tester/Dockerfile | Bumped default Rust toolchain to 1.87.0 |
| atoma-daemon/src/components/openapi.rs | Switched println!("{:?}") to {} with display() |
| Dockerfile | Bumped default Rust toolchain to 1.87.0 |
| .github/workflows/coverage.yml | Changed CI toolchain to stable |
| .github/workflows/ci.yml | Changed CI toolchain to stable |
| pub fn validate_node_message_country_url_timestamp( | ||
| node_message: &NodeMessage, | ||
| ) -> Result<(), AtomaP2pNodeError> { | ||
| ) -> Result<(), Box<AtomaP2pNodeError>> { |
There was a problem hiding this comment.
Use the crate-wide Result<T> alias (which defaults to Box<AtomaP2pNodeError>) instead of spelling out Box<AtomaP2pNodeError> explicitly for consistency.
| ) -> Result<(), Box<AtomaP2pNodeError>> { | |
| ) -> Result<()> { |
| fn validate_country_code(code: &str) -> Result<(), Box<AtomaP2pNodeError>> { | ||
| isocountry::CountryCode::for_alpha2(code).map_err(|_| { | ||
| AtomaP2pNodeError::InvalidCountryCodeError("Country code is invalid.".to_string()) | ||
| Box::new(AtomaP2pNodeError::InvalidCountryCodeError( | ||
| "Country code is invalid.".to_string(), | ||
| )) |
There was a problem hiding this comment.
Similarly, switch this signature to use the crate's Result<T> alias to keep error types consistent across the module.
| let mut buffer = BytesMut::new(); | ||
| ciborium::into_writer(self, (&mut buffer).writer()) | ||
| .map_err(AtomaP2pNodeError::UsageMetricsSerializeError)?; | ||
| ciborium::into_writer(self, (&mut buffer).writer()).map_err(|e| Box::new(e.into()))?; |
There was a problem hiding this comment.
This map_err(|e| Box::new(e.into())) can be simplified to .map_err(Into::into)? because Into::into will convert the error into the correct type and ? will box it via the alias.
| ciborium::into_writer(self, (&mut buffer).writer()).map_err(|e| Box::new(e.into()))?; | |
| ciborium::into_writer(self, (&mut buffer).writer()).map_err(Into::into)?; |
| // Directly deserialize SignedNodeMessage using new method | ||
| let signed_node_message = SignedNodeMessage::deserialize_with_signature(&message_data)?; | ||
| let signed_node_message = | ||
| SignedNodeMessage::deserialize_with_signature(&message_data).map_err(|e| *e)?; |
There was a problem hiding this comment.
[nitpick] Consider replacing map_err(|e| *e)? with map_err(Box::into_inner)? for clarity when unboxing the error.
| SignedNodeMessage::deserialize_with_signature(&message_data).map_err(|e| *e)?; | |
| SignedNodeMessage::deserialize_with_signature(&message_data).map_err(Box::into_inner)?; |
| /// kills the connection before the final chunk is sent. If, instead, | ||
| /// the last chunk is handled, the value is updated to the actual number of tokens | ||
| /// returned by the LLM inference service | ||
| #[allow(clippy::struct_field_names)] |
There was a problem hiding this comment.
[nitpick] If possible, add a brief comment explaining why this lint is suppressed or consider renaming the field instead of disabling the Clippy check for future clarity.
| #[allow(clippy::struct_field_names)] | |
| #[allow(clippy::struct_field_names)] // Suppressed to maintain consistency with existing naming conventions in the project. |
* fix: ensure client errors are correctly tracked (#635) * fix: ensure client errors are correctly tracked * chore: update error tracking * chore: adjust clippy * chore: grammatical error * ci: use stable toolchain (#645) * ci: use stable toolchain * chore: fix clippy issues * revert to use prometheus for queued requests (#646) * revert to use prometheus for queued requests * add start metrics collector * update logs * feat: turn on too many requests for a period of time (#647) * feat: add request running cap (#649) * feat: add request running cap * fix clippy --------- Co-authored-by: Jorge Antonio <matroid@outlook.com> * refactor num running requests for prometheus check * logs * handle deadlock for too many requests timeout trigger check (#650) * feat: add mem usage (#651) * feat: add memusage to get_metrics * add lower threshold for disabling the flag * fix clippy * address 2 comments * add values to config * fix * fix tests * fix name * feat: update sui dependencies (#654) * resolve compilation issues * ci: add caching strategy for ci * ci: optimize coverage job * ci: adjust coverage job * ci: update deny action * ci: use grcov * ci: use stable toolchain * ci: only run tests once * ci: move coverage to test file * ci: use --codecov flag & stable toolchain * ci: discard p2p tester --------- Co-authored-by: chad <chad.nehemiah94@gmail.com> * feat: add max number of queued requests configuration and update request handling (#656) * fix: correct deadlock in `check_if_too_many_requests` (#658) * correct deadlock in check_if_too_many_requests method * resolve tests * add changes * add changes * continue improving logic * add changes * fix: normalize model strings to lowercase in request handlers (#661) * fix: normalize model strings to lowercase in request handlers * fix test * fix --------- Co-authored-by: Chad Nehemiah <chad.nehemiah94@gmail.com> Co-authored-by: Martin Stefcek <35243812+Cifko@users.noreply.github.com>
This upgrades the rust toolchain to the latest stable i.e.
1.87.0Which can be upgraded locally via
rustup