Skip to content

ci: use stable toolchain#645

Merged
jorgeantonio21 merged 2 commits intorelease/v0.1.14from
mc/ci/use-stable-toolchain
May 29, 2025
Merged

ci: use stable toolchain#645
jorgeantonio21 merged 2 commits intorelease/v0.1.14from
mc/ci/use-stable-toolchain

Conversation

@maschad
Copy link
Member

@maschad maschad commented May 28, 2025

This upgrades the rust toolchain to the latest stable i.e. 1.87.0

Which can be upgraded locally via rustup

rustup update stable

@maschad maschad marked this pull request as draft May 28, 2025 19:34
@maschad maschad self-assigned this May 28, 2025
@maschad maschad marked this pull request as ready for review May 28, 2025 21:50
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 in streamer.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>> {
Copy link

Copilot AI May 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use the crate-wide Result<T> alias (which defaults to Box<AtomaP2pNodeError>) instead of spelling out Box<AtomaP2pNodeError> explicitly for consistency.

Suggested change
) -> Result<(), Box<AtomaP2pNodeError>> {
) -> Result<()> {

Copilot uses AI. Check for mistakes.
Comment on lines +117 to +121
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(),
))
Copy link

Copilot AI May 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similarly, switch this signature to use the crate's Result<T> alias to keep error types consistent across the module.

Copilot uses AI. Check for mistakes.
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()))?;
Copy link

Copilot AI May 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Suggested change
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)?;

Copilot uses AI. Check for mistakes.
// 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)?;
Copy link

Copilot AI May 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nitpick] Consider replacing map_err(|e| *e)? with map_err(Box::into_inner)? for clarity when unboxing the error.

Suggested change
SignedNodeMessage::deserialize_with_signature(&message_data).map_err(|e| *e)?;
SignedNodeMessage::deserialize_with_signature(&message_data).map_err(Box::into_inner)?;

Copilot uses AI. Check for mistakes.
/// 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)]
Copy link

Copilot AI May 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[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.

Suggested change
#[allow(clippy::struct_field_names)]
#[allow(clippy::struct_field_names)] // Suppressed to maintain consistency with existing naming conventions in the project.

Copilot uses AI. Check for mistakes.
@jorgeantonio21 jorgeantonio21 merged commit 15e31ba into release/v0.1.14 May 29, 2025
10 checks passed
jorgeantonio21 added a commit that referenced this pull request Jun 5, 2025
* 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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants