Skip to content
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.

Sign up for GitHub

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

Jump to bottom

feat: show loading progress #197

Open
wants to merge 8 commits into
base: main
Choose a base branch
from
Open

feat: show loading progress #197

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
383a6c4
feat: show loading progress
hunjixin May 22, 2024
29c7fbb
feat: add wipe progress
hunjixin May 23, 2024
c52eaa0
feat: update progress
hunjixin May 30, 2024
056d09d
feat: rename basic_progress to percent
hunjixin May 30, 2024
f9e113f
remove empty line
hunjixin May 30, 2024
103d5bd
feat: update message
hunjixin May 30, 2024
59a6440
feat: make title black and message grey
hunjixin May 30, 2024
2c16bf9
chore: fix view format
hunjixin May 30, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
194 changes: 149 additions & 45 deletions src/backend.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -169,13 +166,41 @@ pub enum LoadingStep {
ConsensusNodeCreatedSuccessfully,
CreatingFarmer,
FarmerCreatedSuccessfully,
WipingFarm {
farm_index: u8,
path: PathBuf,
},
WipingNode {
path: PathBuf,
},
WipingFarm,
Copy link
Member

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 send LoadingStep 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 add percentage as a field on this enum, progress field is not necessary either, step.progress() can be called by UI to render progress accordingly.

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)]
Expand All @@ -201,6 +226,7 @@ pub enum BackendNotification {
#[allow(dead_code)]
Copy link
Member

Choose a reason for hiding this comment

The 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,
Expand Down Expand Up @@ -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(),
Copy link
Member

Choose a reason for hiding this comment

The 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
message: "loading configuration ...".to_string(),
message: "Loading configuration ...".to_string(),

})
.await?;

Expand All @@ -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?;

Expand All @@ -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?;

Expand All @@ -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?;

Expand All @@ -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))
Expand All @@ -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?;

Expand All @@ -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?;

Expand All @@ -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?;

Expand All @@ -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?;

Expand All @@ -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?;

Expand All @@ -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?;

Expand All @@ -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?;

Expand All @@ -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?;

Expand All @@ -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?;

Expand Down Expand Up @@ -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?;

Expand Down Expand Up @@ -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?;

Expand Down Expand Up @@ -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?;

Expand All @@ -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?;

Expand All @@ -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());
Copy link
Member

Choose a reason for hiding this comment

The 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,
Expand All @@ -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)
}

Expand Down Expand Up @@ -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?;

Expand All @@ -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 {
Expand Down Expand Up @@ -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?;

Expand All @@ -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(())
Expand Down
Loading