-
Notifications
You must be signed in to change notification settings - Fork 182
chore: refactor to simplify progress bar code in forest-cli snapshot export
#5911
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.
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
Conversation
WalkthroughThe changes update the snapshot export progress reporting in the CLI. Manual terminal output and custom throughput calculations are replaced by an Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant CLI
participant ProgressBar
participant TokioTask
participant SnapshotExporter
User->>CLI: Initiate snapshot export
CLI->>ProgressBar: Create spinner progress bar
CLI->>TokioTask: Spawn async task to update progress bar
CLI->>SnapshotExporter: Start export RPC call
loop Every second
TokioTask->>ProgressBar: Update position with file size
end
SnapshotExporter-->>CLI: Export complete
CLI->>ProgressBar: Finish and clear
CLI->>TokioTask: Await task completion
CLI-->>User: Snapshot export done
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Possibly related PRs
Note 🔌 MCP (Model Context Protocol) integration is now available in Early Access!Pro users can now connect to remote MCP servers under the Integrations page to get reviews and chat conversations that understand additional development context. 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ 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). (9)
✨ Finishing Touches
🧪 Generate unit tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
0d48d06 to
9bab49b
Compare
akaladarshi
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One small comment and a suggestion
src/cli/subcommands/snapshot_cmd.rs
Outdated
| ) | ||
| .expect("indicatif template must be valid"), | ||
| ).with_message(format!("Exporting {} ...", output_path.display())); | ||
| let pb = std::sync::Arc::new(pb); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think this is needed, ProgressBar is already thread safe.
But you can add this pb.enable_steady_tick(std::time::Duration::from_millis(80)); to make the the progress bar animation little bit smoother.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The progress bar is an Arc around its internal state.
Fixed.
Summary of changes
This PR uses
indicatif ::ProgressBarto simplify progress bar logic inforest-cli snapshot export, as a follow-up of@hanabi1224 I think instead of using this whole block we can do this just by using:
Originally posted by @akaladarshi in #5886 (comment)
Changes introduced in this pull request:
Reference issue to close (if applicable)
Closes
Other information and links
Change checklist
Summary by CodeRabbit
New Features
Style