-
Notifications
You must be signed in to change notification settings - Fork 20
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
feat: show loading progress #197
base: main
Are you sure you want to change the base?
Changes from all commits
383a6c4
29c7fbb
c52eaa0
056d09d
f9e113f
103d5bd
59a6440
2c16bf9
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -148,10 +148,7 @@ impl PieceGetter for WeakPieceGetterWrapper { | |||||
pub enum LoadingStep { | ||||||
LoadingConfiguration, | ||||||
ReadingConfiguration, | ||||||
ConfigurationReadSuccessfully { | ||||||
/// Whether configuration exists, `false` on the first start | ||||||
configuration_exists: bool, | ||||||
}, | ||||||
ConfigurationReadSuccessfully, | ||||||
CheckingConfiguration, | ||||||
ConfigurationIsValid, | ||||||
DecodingChainSpecification, | ||||||
|
@@ -169,13 +166,41 @@ pub enum LoadingStep { | |||||
ConsensusNodeCreatedSuccessfully, | ||||||
CreatingFarmer, | ||||||
FarmerCreatedSuccessfully, | ||||||
WipingFarm { | ||||||
farm_index: u8, | ||||||
path: PathBuf, | ||||||
}, | ||||||
WipingNode { | ||||||
path: PathBuf, | ||||||
}, | ||||||
WipingFarm, | ||||||
WipedFarmSuccessfully, | ||||||
WipingNode, | ||||||
WipedNodeSuccessfully, | ||||||
} | ||||||
|
||||||
impl LoadingStep { | ||||||
fn percentage(&self) -> f32 { | ||||||
match self { | ||||||
LoadingStep::LoadingConfiguration => 0.0, | ||||||
LoadingStep::ReadingConfiguration => 1.0, | ||||||
LoadingStep::ConfigurationReadSuccessfully => 2.0, | ||||||
LoadingStep::CheckingConfiguration => 3.0, | ||||||
LoadingStep::ConfigurationIsValid => 4.0, | ||||||
LoadingStep::DecodingChainSpecification => 5.0, | ||||||
LoadingStep::DecodedChainSpecificationSuccessfully => 7.0, | ||||||
LoadingStep::CheckingNodePath => 9.0, | ||||||
LoadingStep::CreatingNodePath => 10.0, | ||||||
LoadingStep::NodePathReady => 11.0, | ||||||
LoadingStep::PreparingNetworkingStack => 13.0, | ||||||
LoadingStep::ReadingNetworkKeypair => 15.0, | ||||||
LoadingStep::GeneratingNetworkKeypair => 17.0, | ||||||
LoadingStep::WritingNetworkKeypair => 18.0, | ||||||
LoadingStep::InstantiatingNetworkingStack => 19.0, | ||||||
LoadingStep::NetworkingStackCreatedSuccessfully => 20.0, | ||||||
LoadingStep::CreatingConsensusNode => 20.0, | ||||||
LoadingStep::ConsensusNodeCreatedSuccessfully => 40.0, | ||||||
LoadingStep::CreatingFarmer => 40.0, | ||||||
LoadingStep::FarmerCreatedSuccessfully => 100.0, | ||||||
LoadingStep::WipingFarm => 0.0, | ||||||
LoadingStep::WipedFarmSuccessfully => 50.0, | ||||||
LoadingStep::WipingNode => 80.0, | ||||||
LoadingStep::WipedNodeSuccessfully => 100.0, | ||||||
} | ||||||
} | ||||||
} | ||||||
|
||||||
#[derive(Debug)] | ||||||
|
@@ -201,6 +226,7 @@ pub enum BackendNotification { | |||||
#[allow(dead_code)] | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This and above TODO should be removed by this PR |
||||||
/// Progress in %: 0.0..=100.0 | ||||||
progress: f32, | ||||||
message: String, | ||||||
}, | ||||||
IncompatibleChain { | ||||||
raw_config: RawConfig, | ||||||
|
@@ -616,7 +642,8 @@ async fn load_configuration( | |||||
notifications_sender | ||||||
.send(BackendNotification::Loading { | ||||||
step: LoadingStep::LoadingConfiguration, | ||||||
progress: 0.0, | ||||||
progress: LoadingStep::LoadingConfiguration.percentage(), | ||||||
message: "loading configuration ...".to_string(), | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I see all of these messages were copied from another file, but why did you remove capitalization everywhere?:
Suggested change
|
||||||
}) | ||||||
.await?; | ||||||
|
||||||
|
@@ -625,7 +652,8 @@ async fn load_configuration( | |||||
notifications_sender | ||||||
.send(BackendNotification::Loading { | ||||||
step: LoadingStep::ReadingConfiguration, | ||||||
progress: 0.0, | ||||||
progress: LoadingStep::ReadingConfiguration.percentage(), | ||||||
message: "reading configuration ...".to_string(), | ||||||
}) | ||||||
.await?; | ||||||
|
||||||
|
@@ -634,10 +662,16 @@ async fn load_configuration( | |||||
|
||||||
notifications_sender | ||||||
.send(BackendNotification::Loading { | ||||||
step: LoadingStep::ConfigurationReadSuccessfully { | ||||||
configuration_exists: maybe_config.is_some(), | ||||||
}, | ||||||
progress: 0.0, | ||||||
step: LoadingStep::ConfigurationReadSuccessfully.clone(), | ||||||
progress: LoadingStep::ConfigurationReadSuccessfully.percentage(), | ||||||
message: format!( | ||||||
"configuration {}", | ||||||
if maybe_config.is_some() { | ||||||
"found" | ||||||
} else { | ||||||
"not found" | ||||||
} | ||||||
), | ||||||
}) | ||||||
.await?; | ||||||
|
||||||
|
@@ -652,7 +686,8 @@ async fn check_configuration( | |||||
notifications_sender | ||||||
.send(BackendNotification::Loading { | ||||||
step: LoadingStep::CheckingConfiguration, | ||||||
progress: 0.0, | ||||||
progress: LoadingStep::CheckingConfiguration.percentage(), | ||||||
message: "checking configuration ...".to_string(), | ||||||
}) | ||||||
.await?; | ||||||
|
||||||
|
@@ -661,7 +696,8 @@ async fn check_configuration( | |||||
notifications_sender | ||||||
.send(BackendNotification::Loading { | ||||||
step: LoadingStep::ConfigurationIsValid, | ||||||
progress: 0.0, | ||||||
progress: LoadingStep::ConfigurationIsValid.percentage(), | ||||||
message: "configuration is valid".to_string(), | ||||||
}) | ||||||
.await?; | ||||||
Ok(Some(config)) | ||||||
|
@@ -685,7 +721,8 @@ async fn load_chain_specification( | |||||
notifications_sender | ||||||
.send(BackendNotification::Loading { | ||||||
step: LoadingStep::DecodingChainSpecification, | ||||||
progress: 0.0, | ||||||
progress: LoadingStep::DecodingChainSpecification.percentage(), | ||||||
message: "decoding chain specification ...".to_string(), | ||||||
}) | ||||||
.await?; | ||||||
|
||||||
|
@@ -695,7 +732,8 @@ async fn load_chain_specification( | |||||
notifications_sender | ||||||
.send(BackendNotification::Loading { | ||||||
step: LoadingStep::DecodedChainSpecificationSuccessfully, | ||||||
progress: 0.0, | ||||||
progress: LoadingStep::DecodedChainSpecificationSuccessfully.percentage(), | ||||||
message: "decoded chain specification successfully".to_string(), | ||||||
}) | ||||||
.await?; | ||||||
|
||||||
|
@@ -709,7 +747,8 @@ async fn preparing_node_path( | |||||
notifications_sender | ||||||
.send(BackendNotification::Loading { | ||||||
step: LoadingStep::CheckingNodePath, | ||||||
progress: 0.0, | ||||||
progress: LoadingStep::CheckingNodePath.percentage(), | ||||||
message: "checking node path ...".to_string(), | ||||||
}) | ||||||
.await?; | ||||||
|
||||||
|
@@ -723,7 +762,8 @@ async fn preparing_node_path( | |||||
notifications_sender | ||||||
.send(BackendNotification::Loading { | ||||||
step: LoadingStep::CreatingNodePath, | ||||||
progress: 0.0, | ||||||
progress: LoadingStep::CreatingNodePath.percentage(), | ||||||
message: "creating node path ...".to_string(), | ||||||
}) | ||||||
.await?; | ||||||
|
||||||
|
@@ -739,7 +779,8 @@ async fn preparing_node_path( | |||||
notifications_sender | ||||||
.send(BackendNotification::Loading { | ||||||
step: LoadingStep::NodePathReady, | ||||||
progress: 0.0, | ||||||
progress: LoadingStep::NodePathReady.percentage(), | ||||||
message: "node path ready".to_string(), | ||||||
}) | ||||||
.await?; | ||||||
|
||||||
|
@@ -763,7 +804,8 @@ async fn create_networking_stack( | |||||
notifications_sender | ||||||
.send(BackendNotification::Loading { | ||||||
step: LoadingStep::PreparingNetworkingStack, | ||||||
progress: 0.0, | ||||||
progress: LoadingStep::PreparingNetworkingStack.percentage(), | ||||||
message: "preparing networking stack ...".to_string(), | ||||||
}) | ||||||
.await?; | ||||||
|
||||||
|
@@ -782,7 +824,8 @@ async fn create_networking_stack( | |||||
notifications_sender | ||||||
.send(BackendNotification::Loading { | ||||||
step: LoadingStep::ReadingNetworkKeypair, | ||||||
progress: 0.0, | ||||||
progress: LoadingStep::ReadingNetworkKeypair.percentage(), | ||||||
message: "reading network keypair ....".to_string(), | ||||||
}) | ||||||
.await?; | ||||||
|
||||||
|
@@ -799,7 +842,8 @@ async fn create_networking_stack( | |||||
notifications_sender | ||||||
.send(BackendNotification::Loading { | ||||||
step: LoadingStep::GeneratingNetworkKeypair, | ||||||
progress: 0.0, | ||||||
progress: LoadingStep::GeneratingNetworkKeypair.percentage(), | ||||||
message: "generating network keypair ...".to_string(), | ||||||
}) | ||||||
.await?; | ||||||
|
||||||
|
@@ -808,7 +852,8 @@ async fn create_networking_stack( | |||||
notifications_sender | ||||||
.send(BackendNotification::Loading { | ||||||
step: LoadingStep::WritingNetworkKeypair, | ||||||
progress: 0.0, | ||||||
progress: LoadingStep::WritingNetworkKeypair.percentage(), | ||||||
message: "writing network keypair ...".to_string(), | ||||||
}) | ||||||
.await?; | ||||||
|
||||||
|
@@ -848,7 +893,8 @@ async fn create_networking_stack( | |||||
notifications_sender | ||||||
.send(BackendNotification::Loading { | ||||||
step: LoadingStep::InstantiatingNetworkingStack, | ||||||
progress: 0.0, | ||||||
progress: LoadingStep::InstantiatingNetworkingStack.percentage(), | ||||||
message: "instantiating networking stack ...".to_string(), | ||||||
}) | ||||||
.await?; | ||||||
|
||||||
|
@@ -889,7 +935,8 @@ async fn create_networking_stack( | |||||
notifications_sender | ||||||
.send(BackendNotification::Loading { | ||||||
step: LoadingStep::NetworkingStackCreatedSuccessfully, | ||||||
progress: 0.0, | ||||||
progress: LoadingStep::NetworkingStackCreatedSuccessfully.percentage(), | ||||||
message: "created networking stack successfully".to_string(), | ||||||
}) | ||||||
.await?; | ||||||
|
||||||
|
@@ -917,7 +964,8 @@ async fn create_consensus_node( | |||||
notifications_sender | ||||||
.send(BackendNotification::Loading { | ||||||
step: LoadingStep::CreatingConsensusNode, | ||||||
progress: 0.0, | ||||||
progress: LoadingStep::CreatingConsensusNode.percentage(), | ||||||
message: "creating consensus node ...".to_string(), | ||||||
}) | ||||||
.await?; | ||||||
|
||||||
|
@@ -943,7 +991,8 @@ async fn create_consensus_node( | |||||
notifications_sender | ||||||
.send(BackendNotification::Loading { | ||||||
step: LoadingStep::ConsensusNodeCreatedSuccessfully, | ||||||
progress: 0.0, | ||||||
progress: LoadingStep::ConsensusNodeCreatedSuccessfully.percentage(), | ||||||
message: "created consensus node successfully".to_string(), | ||||||
}) | ||||||
.await?; | ||||||
|
||||||
|
@@ -965,10 +1014,46 @@ async fn create_farmer( | |||||
notifications_sender | ||||||
.send(BackendNotification::Loading { | ||||||
step: LoadingStep::CreatingFarmer, | ||||||
progress: 0.0, | ||||||
progress: LoadingStep::CreatingFarmer.percentage(), | ||||||
message: "start to create farmer ...".to_string(), | ||||||
}) | ||||||
.await?; | ||||||
|
||||||
let percent_per_farm = (LoadingStep::FarmerCreatedSuccessfully.percentage() | ||||||
- LoadingStep::FarmerCreatedSuccessfully.percentage()) | ||||||
/ (disk_farms.len() as f32); | ||||||
|
||||||
let notifications = Arc::new(farmer::Notifications::default()); | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please don't do this, this is not not notifications are supposed to work. There are already subscriptions for existing notifications, just follow that pattern instead of breaking abstractions. |
||||||
let on_create_farmer_notification_handler_id = notifications.add({ | ||||||
let notifications_sender = notifications_sender.clone(); | ||||||
|
||||||
Arc::new(move |notification| { | ||||||
let mut notifications_sender = notifications_sender.clone(); | ||||||
|
||||||
if let farmer::FarmerNotification::FarmingLog { | ||||||
farm_index, | ||||||
message, | ||||||
} = notification | ||||||
{ | ||||||
if let Err(error) = notifications_sender | ||||||
.try_send(BackendNotification::Loading { | ||||||
step: LoadingStep::CreatingFarmer, | ||||||
progress: percent_per_farm * *farm_index as f32, | ||||||
message: message.clone(), | ||||||
}) | ||||||
.or_else(|error| { | ||||||
tokio::task::block_in_place(|| { | ||||||
Handle::current() | ||||||
.block_on(notifications_sender.send(error.into_inner())) | ||||||
}) | ||||||
}) | ||||||
{ | ||||||
warn!(%error, "Failed to send creating farmer backend notification"); | ||||||
} | ||||||
} | ||||||
}) | ||||||
}); | ||||||
|
||||||
let farmer_options = FarmerOptions { | ||||||
reward_address, | ||||||
disk_farms, | ||||||
|
@@ -978,17 +1063,19 @@ async fn create_farmer( | |||||
farmer_cache_worker, | ||||||
kzg, | ||||||
piece_getter, | ||||||
notifications, | ||||||
}; | ||||||
|
||||||
let farmer = farmer::create_farmer(farmer_options).await?; | ||||||
|
||||||
on_create_farmer_notification_handler_id.detach(); | ||||||
notifications_sender | ||||||
.send(BackendNotification::Loading { | ||||||
step: LoadingStep::FarmerCreatedSuccessfully, | ||||||
progress: 0.0, | ||||||
progress: LoadingStep::FarmerCreatedSuccessfully.percentage(), | ||||||
message: "created farmer successfully".to_string(), | ||||||
}) | ||||||
.await?; | ||||||
|
||||||
Ok(farmer) | ||||||
} | ||||||
|
||||||
|
@@ -1034,13 +1121,15 @@ pub async fn wipe( | |||||
let farms = raw_config.farms(); | ||||||
for (farm_index, farm) in farms.iter().enumerate() { | ||||||
let path = &farm.path; | ||||||
let percent_per_farm = (LoadingStep::WipedFarmSuccessfully.percentage() | ||||||
- LoadingStep::WipingFarm.percentage()) | ||||||
/ (farms.len() as f32); | ||||||
notifications_sender | ||||||
.send(BackendNotification::Loading { | ||||||
step: LoadingStep::WipingFarm { | ||||||
farm_index: farm_index as u8, | ||||||
path: path.to_path_buf(), | ||||||
}, | ||||||
progress: 0.0, | ||||||
step: LoadingStep::WipingFarm, | ||||||
progress: LoadingStep::WipingFarm.percentage() | ||||||
+ farm_index as f32 * percent_per_farm, | ||||||
message: format!("wiping farm {farm_index} at {}...", path.display()), | ||||||
}) | ||||||
.await?; | ||||||
|
||||||
|
@@ -1051,7 +1140,15 @@ pub async fn wipe( | |||||
}); | ||||||
|
||||||
match wipe_fut.await { | ||||||
Ok(Ok(())) => {} | ||||||
Ok(Ok(())) => { | ||||||
notifications_sender | ||||||
.send(BackendNotification::Loading { | ||||||
step: LoadingStep::WipedFarmSuccessfully, | ||||||
progress: LoadingStep::WipedFarmSuccessfully.percentage(), | ||||||
message: "wiped farm successfully".to_string(), | ||||||
}) | ||||||
.await?; | ||||||
} | ||||||
Ok(Err(error)) => { | ||||||
notifications_sender | ||||||
.send(BackendNotification::IrrecoverableError { | ||||||
|
@@ -1079,10 +1176,9 @@ pub async fn wipe( | |||||
let path = &raw_config.node_path(); | ||||||
notifications_sender | ||||||
.send(BackendNotification::Loading { | ||||||
step: LoadingStep::WipingNode { | ||||||
path: path.to_path_buf(), | ||||||
}, | ||||||
progress: 0.0, | ||||||
step: LoadingStep::WipingNode, | ||||||
progress: LoadingStep::WipingNode.percentage(), | ||||||
message: format!("wiping node at {}...", path.display()), | ||||||
}) | ||||||
.await?; | ||||||
|
||||||
|
@@ -1103,6 +1199,14 @@ pub async fn wipe( | |||||
} | ||||||
} | ||||||
} | ||||||
|
||||||
notifications_sender | ||||||
.send(BackendNotification::Loading { | ||||||
step: LoadingStep::WipedNodeSuccessfully, | ||||||
progress: LoadingStep::WipedNodeSuccessfully.percentage(), | ||||||
message: "wiped node successfully".to_string(), | ||||||
}) | ||||||
.await?; | ||||||
} | ||||||
|
||||||
Ok(()) | ||||||
|
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 it is the right change to remove properties from this enum variant. In fact I think more variants should have more properties here!
Instead of sending
message
to the frontend, you should sendLoadingStep
such that there is enough information for frontend to render the message (kind of like it was before). Backend should not be responsible to rendering user-facing messages for frontend, there is a separation of responsibilities here.step
should contain all the information necessary. Moreover, since you have decided to addpercentage
as a field on this enum,progress
field is not necessary either,step.progress()
can be called by UI to render progress accordingly.