8000 chore(supervisor): error logs consistency by dhyaniarun1993 · Pull Request #1889 · op-rs/kona · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

chore(supervisor): error logs consistency #1889

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

8000

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 8 commits into from
May 29, 2025

Conversation

dhyaniarun1993
Copy link
Collaborator

No description provided.

@dhyaniarun1993 dhyaniarun1993 self-assigned this May 28, 2025
@dhyaniarun1993 dhyaniarun1993 added K-chore Kind: chore W-supervisor Workstream: supervisor labels May 28, 2025
Copy link
codecov bot commented May 28, 2025

Codecov Report

Attention: Patch coverage is 69.69697% with 10 lines in your changes missing coverage. Please review.

Project coverage is 83.8%. Comparing base (45a74c4) to head (a5eda40).
Report is 3 commits behind head on main.

✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
...s/supervisor/storage/src/providers/log_provider.rs 64.0% 9 Missing ⚠️
...visor/storage/src/providers/derivation_provider.rs 66.6% 1 Missing ⚠️

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copy link
Member
@emhane emhane left a comment

Choose a reason for hiding this comment

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

smol comments

Comment on lines 22 to 24
#[display(
"BlockRef {{ number: {number}, hash: {hash}, parent_hash: {parent_hash}, timestamp: {timestamp} }}"
)]
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
#[display(
"BlockRef {{ number: {number}, hash: {hash}, parent_hash: {parent_hash}, timestamp: {timestamp} }}"
)]
#[display(
"number: {number}, hash: {hash}, parent_hash: {parent_hash}, timestamp: {timestamp}"
)]

this looks like the debug format, the suggested format is expected for display, see docs https://doc.rust-lang.org/rust-by-example/hello/print/print_display.html

})?;

let block = block_option.ok_or_else(|| {
warn!(target: "supervisor_storage", block_number, "Block not found");
StorageError::EntryNotFound(format!("Block {} not found", block_number))
StorageError::EntryNotFound(format!("block {} not found", block_number))
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
StorageError::EntryNotFound(format!("block {} not found", block_number))
StorageError::EntryNotFound(format!("block {block_number} not found"))

Comment on lines 164 to 165
"log not found at block {} index {}",
block_number, log.index
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
"log not found at block {} index {}",
block_number, log.index
"log not found at block {block_number} index {}",
log.index

Comment on lines +48 to +49
/// This method is append only and does not overwrite existing pairs.
/// Ensures that the latest stored pair is the parent of the incoming pair before saving.
Copy link
Member

Choose a reason for hiding this comment

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

nice!

@emhane emhane added the K-debt Kind: debt label May 28, 2025
@dhyaniarun1993 dhyaniarun1993 marked this pull request as ready for review May 28, 2025 17:14
@dhyaniarun1993 dhyaniarun1993 added this pull request to the merge queue May 29, 2025
Merged via the queue into main with commit 6498110 May 29, 2025
22 of 23 checks passed
@dhyaniarun1993 dhyaniarun1993 deleted the chore/error-logs-consistency branch May 29, 2025 03:33
@github-project-automation github-project-automation bot moved this from In Review to Done in Project Tracking May 29, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
K-chore Kind: chore K-debt Kind: debt W-supervisor Workstream: supervisor
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

3 participants
0