P2poolv2 status view#13
Conversation
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
There was a problem hiding this comment.
Pull request overview
This PR adds P2Pool v2 integration to the PDM (P2Pool Data Manager) TUI application. It introduces live status monitoring via a metrics API endpoint and adds configuration file management for P2Pool configs. The implementation includes a new config parser, metrics fetching via a background task, and UI screens for config viewing and live status.
Changes:
- Added P2Pool config parser with TOML parsing and validation
- Implemented live metrics fetching from P2Pool API using async HTTP requests
- Added two new UI screens: P2Pool Config viewer and P2Pool Status monitor
- Refactored input handling to use an AppAction enum pattern
Reviewed changes
Copilot reviewed 13 out of 14 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| src/p2poolv2_config_parser.rs | New module for parsing and validating P2Pool v2 TOML configs with comprehensive tests |
| src/components/metrics.rs | New module for parsing Prometheus-format metrics from P2Pool API |
| src/main.rs | Converted to async with tokio, added background metrics fetcher task and event polling |
| src/ui.rs | Added P2Pool Config and Status views, plus render functions for both configs |
| src/app.rs | Added AppAction enum, P2Pool state fields, and new screen variants |
| src/components/file_explorer.rs | Added handle_input method returning AppAction |
| Cargo.toml | Added tokio, reqwest, bitcoin, and serde dependencies |
| tests/snapshots/* | Updated snapshots to reflect new sidebar items |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if let Some(config) = &app.p2pool_conf_path { | ||
| // dynamic override: | ||
| // let host = &config.api_host; | ||
| // let port = config.api_port; | ||
|
|
||
| let host = "127.0.0.1"; | ||
| let port = 46884; |
There was a problem hiding this comment.
The API URL construction is hardcoded and the parsed P2PoolConfig is not stored in App state. The config is parsed into ConfigEntry list for display but the original P2PoolConfig struct (which contains the ApiConfig) is discarded. To dynamically extract the API host and port from the loaded config, you need to either: 1) Store the parsed P2PoolConfig in App state, or 2) Parse it again here from the path to extract the API settings.
| if let Some(config) = &app.p2pool_conf_path { | |
| // dynamic override: | |
| // let host = &config.api_host; | |
| // let port = config.api_port; | |
| let host = "127.0.0.1"; | |
| let port = 46884; | |
| if let Some(config_path) = &app.p2pool_conf_path { | |
| // Try to derive API host and port from the selected P2Pool config. | |
| // Fall back to sensible defaults if entries are missing or invalid. | |
| let mut host: Option<String> = None; | |
| let mut port: Option<u16> = None; | |
| if let Ok(entries) = parse_p2pool_config(config_path) { | |
| for entry in entries { | |
| // NOTE: field and key names follow the parser's ConfigEntry semantics. | |
| // Unknown or unexpected keys are ignored. | |
| #[allow(clippy::match_single_binding)] | |
| match entry.key.as_str() { | |
| "api_host" => { | |
| host = Some(entry.value.clone()); | |
| } | |
| "api_port" => { | |
| if let Ok(parsed) = entry.value.parse::<u16>() { | |
| port = Some(parsed); | |
| } | |
| } | |
| _ => {} | |
| } | |
| } | |
| } | |
| let host = host.unwrap_or_else(|| "127.0.0.1".to_string()); | |
| let port = port.unwrap_or(46884); |
| serde = { version = "1.0", features = ["derive"] } | ||
| bitcoin = "0.31" | ||
| tokio = { version = "1.49.0", features = ["full"] } | ||
| reqwest = { version = "0.13.2", features = ["blocking"] } |
There was a problem hiding this comment.
The reqwest dependency is configured with the "blocking" feature, but the code uses the async API (client.get().send().await). The "blocking" feature is for synchronous HTTP requests. For async usage, you should remove "blocking" and keep only the default async features, or use no explicit features at all as the async API is the default.
| reqwest = { version = "0.13.2", features = ["blocking"] } | |
| reqwest = "0.13.2" |
| if let Ok(entries) = parse_p2pool_config(&path) { | ||
| app.p2pool_data = entries; | ||
| } |
There was a problem hiding this comment.
Config parsing errors are silently ignored. If parse_p2pool_config fails, the path is still set but p2pool_data remains empty, leading to a confusing UI state where a file is loaded but nothing is displayed. Consider storing the parse error in App state and showing it to the user, or at least logging the error.
| if let Ok(entries) = parse_bitcoin_config(&path) { | ||
| app.bitcoin_data = entries; | ||
| } |
There was a problem hiding this comment.
Config parsing errors are silently ignored. If parse_bitcoin_config fails, the path is still set but bitcoin_data remains empty, leading to a confusing UI state where a file is loaded but nothing is displayed. Consider storing the parse error in App state and showing it to the user, or at least logging the error.
| if let Ok(resp) = client.get(&u).send().await { | ||
| if let Ok(text) = resp.text().await { | ||
| let metrics = P2PoolMetrics::parse_prometheus(&text); | ||
| // Send them to the UI thread | ||
| let _ = tx.send(metrics); | ||
| } | ||
| } |
There was a problem hiding this comment.
The HTTP request to fetch metrics has no timeout configured. If the p2pool node is unresponsive, this could hang for a very long time (default TCP timeout is typically 30-120 seconds). Consider adding a timeout using reqwest's timeout() method or tokio's timeout() wrapper to ensure the background task remains responsive even when the node is down.
| && key.kind == KeyEventKind::Press | ||
| { | ||
| if key.code == KeyCode::Char('q') { | ||
| if let Event::Key(key) = event::read()? { |
There was a problem hiding this comment.
The event_handler parameter passed to run_app is never used. This line directly calls event::read() instead of using the event_handler closure that was set up with polling logic. This causes the application to block indefinitely waiting for keyboard input, preventing the metrics from updating in the UI even when they arrive via the channel. Replace this with: if let Some(Event::Key(key)) = event_handler(app)? {
| if let Event::Key(key) = event::read()? { | |
| if let Some(Event::Key(key)) = event_handler(app)? { |
integrate p2pool api and status view.