Skip to content

Commit

Permalink
refactor(cli): integrate fs-err library for file operations (#1255)
Browse files Browse the repository at this point in the history
  • Loading branch information
sasa-tomic authored Jan 28, 2025
1 parent 3e25721 commit 2291c60
Show file tree
Hide file tree
Showing 32 changed files with 94 additions and 65 deletions.
22 changes: 21 additions & 1 deletion Cargo.Bazel.lock
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
{
"checksum": "601d274f5413afdb0d91d2bc6012fa2ee15de9b5f6b3cff8d89c08768cdd4184",
"checksum": "cc45b1bfc5914f24e89a7c8c6021bf9e0959d61987b2904e57c93991572fa7c8",
"crates": {
"actix-codec 0.5.2": {
"name": "actix-codec",
Expand Down Expand Up @@ -34715,6 +34715,10 @@
"id": "crossbeam-channel 0.5.14",
"target": "crossbeam_channel"
},
{
"id": "fs-err 3.0.0",
"target": "fs_err"
},
{
"id": "futures 0.3.31",
"target": "futures"
Expand Down Expand Up @@ -34837,6 +34841,10 @@
"id": "crossbeam-channel 0.5.14",
"target": "crossbeam_channel"
},
{
"id": "fs-err 3.0.0",
"target": "fs_err"
},
{
"id": "futures-util 0.3.31",
"target": "futures_util"
Expand Down Expand Up @@ -47497,6 +47505,10 @@
"id": "crossbeam-channel 0.5.14",
"target": "crossbeam_channel"
},
{
"id": "fs-err 3.0.0",
"target": "fs_err"
},
{
"id": "futures 0.3.31",
"target": "futures"
Expand Down Expand Up @@ -48224,6 +48236,10 @@
"id": "env_logger 0.11.6",
"target": "env_logger"
},
{
"id": "fs-err 3.0.0",
"target": "fs_err"
},
{
"id": "futures 0.3.31",
"target": "futures"
Expand Down Expand Up @@ -48992,6 +49008,10 @@
"id": "crossbeam-channel 0.5.14",
"target": "crossbeam_channel"
},
{
"id": "fs-err 3.0.0",
"target": "fs_err"
},
{
"id": "futures-util 0.3.31",
"target": "futures_util"
Expand Down
5 changes: 5 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion rs/cli/src/artifact_downloader.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ pub trait ArtifactDownloader: Sync + Send {
let download_dir = format!("{}/tmp/ic/{}", dirs::home_dir().expect("home_dir is not set").as_path().display(), subdir);
let download_dir = Path::new(&download_dir);

std::fs::create_dir_all(download_dir).unwrap_or_else(|_| panic!("create_dir_all failed for {}", download_dir.display()));
fs_err::create_dir_all(download_dir).unwrap_or_else(|_| panic!("create_dir_all failed for {}", download_dir.display()));

let download_image = format!("{}/update-img.tar.gz", download_dir.to_str().unwrap());
let download_image = Path::new(&download_image);
Expand Down
4 changes: 2 additions & 2 deletions rs/cli/src/auth.rs
Original file line number Diff line number Diff line change
Expand Up @@ -207,13 +207,13 @@ impl Neuron {

let parent = path.parent().ok_or(anyhow::anyhow!("Expected parent to exist"))?;
if !parent.exists() {
std::fs::create_dir_all(parent)?
fs_err::create_dir_all(parent)?
}

let key_pair = rosetta_core::models::Ed25519KeyPair::generate(42);

if !path.exists() {
std::fs::write(&path, key_pair.to_pem())?;
fs_err::write(&path, key_pair.to_pem())?;
}
Ok(path)
}
Expand Down
2 changes: 1 addition & 1 deletion rs/cli/src/commands/der_to_principal.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ impl ExecutableCommand for DerToPrincipal {
}

async fn execute(&self, _ctx: crate::ctx::DreContext) -> anyhow::Result<()> {
let principal = ic_base_types::PrincipalId::new_self_authenticating(&std::fs::read(&self.path)?);
let principal = ic_base_types::PrincipalId::new_self_authenticating(&fs_err::read(&self.path)?);
println!("{}", principal);
Ok(())
}
Expand Down
2 changes: 1 addition & 1 deletion rs/cli/src/commands/update_authorized_subnets.rs
Original file line number Diff line number Diff line change
Expand Up @@ -183,7 +183,7 @@ impl ExecutableCommand for UpdateAuthorizedSubnets {
impl UpdateAuthorizedSubnets {
fn parse_csv(&self) -> anyhow::Result<Vec<(String, String)>> {
let contents = match &self.path {
Some(p) => std::fs::read_to_string(p)?,
Some(p) => fs_err::read_to_string(p)?,
None => {
info!("Using embedded version of authorized subnets csv that is added during build time");
DEFAULT_AUTHORIZED_SUBNETS_CSV.to_string()
Expand Down
12 changes: 6 additions & 6 deletions rs/cli/src/commands/upgrade.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ impl Upgrade {
let update_check_path = dirs::cache_dir().expect("Failed to find a cache dir").join("dre_update_check");
if !proceed_with_upgrade {
// Check for a new release once per day
if let Ok(metadata) = std::fs::metadata(&update_check_path) {
if let Ok(metadata) = fs_err::metadata(&update_check_path) {
let last_check = metadata.modified().unwrap();
let now = std::time::SystemTime::now();
if now.duration_since(last_check).unwrap().as_secs() < 60 * 60 * 24 {
Expand All @@ -57,7 +57,7 @@ impl Upgrade {
.map_err(|e| anyhow::anyhow!("Configuring backend failed: {:?}", e))?;

// Touch update check file
std::fs::write(&update_check_path, "").map_err(|e| anyhow::anyhow!("Couldn't touch update check file: {:?}", e))?;
fs_err::write(&update_check_path, "").map_err(|e| anyhow::anyhow!("Couldn't touch update check file: {:?}", e))?;

let releases = maybe_configured_backend
.fetch()
Expand Down Expand Up @@ -105,8 +105,8 @@ impl Upgrade {

let new_dre_path = tmp_dir.path().join(&asset.name);
let asset_path = tmp_dir.path().join("asset");
let asset_file = std::fs::File::create(&asset_path).map_err(|e| anyhow::anyhow!("Couldn't create file: {:?}", e))?;
let new_dre_file = std::fs::File::create(&new_dre_path).map_err(|e| anyhow::anyhow!("Couldn't create file: {:?}", e))?;
let asset_file = fs_err::File::create(&asset_path).map_err(|e| anyhow::anyhow!("Couldn't create file: {:?}", e))?;
let new_dre_file = fs_err::File::create(&new_dre_path).map_err(|e| anyhow::anyhow!("Couldn't create file: {:?}", e))?;

self_update::Download::from_url(&asset.download_url)
.show_progress(true)
Expand All @@ -116,7 +116,7 @@ impl Upgrade {
info!("Asset downloaded successfully");

let value: Value =
serde_json::from_str(&std::fs::read_to_string(&asset_path).unwrap()).map_err(|e| anyhow::anyhow!("Couldn't open asset: {:?}", e))?;
serde_json::from_str(&fs_err::read_to_string(&asset_path).unwrap()).map_err(|e| anyhow::anyhow!("Couldn't open asset: {:?}", e))?;

let download_url = match value.get("browser_download_url") {
Some(Value::String(d)) => d,
Expand All @@ -134,7 +134,7 @@ impl Upgrade {
// Since its possible to upgrade to an older version
// remove the metafile so that the check will be run
// with the new version again
std::fs::remove_file(&update_check_path)?;
fs_err::remove_file(&update_check_path)?;

Ok(UpdateStatus::Updated(release.version.clone()))
}
Expand Down
4 changes: 2 additions & 2 deletions rs/cli/src/cordoned_feature_fetcher.rs
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ impl CordonedFeatureFetcherImpl {
.bytes()
.await?;

if let Err(e) = std::fs::write(&self.local_copy, &bytes) {
if let Err(e) = fs_err::write(&self.local_copy, &bytes) {
warn!(
"Failed to update cordoned features cache on path `{}` due to: {:?}",
self.local_copy.display(),
Expand All @@ -63,7 +63,7 @@ impl CordonedFeatureFetcherImpl {
}

fn fetch_from_file(&self) -> anyhow::Result<Vec<CordonedFeature>> {
let contents = std::fs::read(&self.local_copy)?;
let contents = fs_err::read(&self.local_copy)?;

self.parse(&contents)
}
Expand Down
2 changes: 1 addition & 1 deletion rs/cli/src/discourse_client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -257,7 +257,7 @@ fn get_subnet_topics_map() -> BTreeMap<PrincipalId, SubnetTopicInfo> {
}

fn get_subnet_topics_from_path(path: &Path) -> anyhow::Result<BTreeMap<PrincipalId, SubnetTopicInfo>> {
let file = std::fs::File::open(path)?;
let file = fs_err::File::open(path)?;
serde_json::from_reader(file).map_err(anyhow::Error::from)
}

Expand Down
2 changes: 1 addition & 1 deletion rs/cli/src/qualification/run_xnet_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ impl Step for RunXnetTest {
if !key.exists() {
anyhow::bail!("Principal key for xnet testing not found at {}", key.display());
}
let file = std::fs::File::open(&key)?;
let file = fs_err::File::open(&key)?;
file.set_permissions(PermissionsExt::from_mode(0o400))?;

let e2e_bin = ctx.download_executable(E2E_TEST_DRIVER, &self.version).await?;
Expand Down
14 changes: 7 additions & 7 deletions rs/cli/src/qualification/util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ pub struct StepCtx {
impl StepCtx {
pub fn new(dre_ctx: DreContext, artifacts: Option<PathBuf>, from_version: String, to_version: String) -> anyhow::Result<Self> {
let artifacts_of_run = artifacts.as_ref().map(|t| {
if let Err(e) = std::fs::create_dir_all(t) {
if let Err(e) = fs_err::create_dir_all(t) {
panic!("Couldn't create dir {}: {:?}", t.display(), e)
}
t.clone()
Expand Down Expand Up @@ -62,13 +62,13 @@ impl StepCtx {
pub async fn download_canister(&self, canister: &str, version: &str) -> anyhow::Result<PathBuf> {
let cache = dirs::cache_dir().ok_or(anyhow::anyhow!("Can't cache dir"))?.join(IC_EXECUTABLES_DIR);
if !cache.exists() {
std::fs::create_dir_all(&cache)?;
fs_err::create_dir_all(&cache)?;
}

let artifact_path = cache.join(format!("{}/{}.{}", canister, canister, version));
let artifact_dir = artifact_path.parent().unwrap();
if !artifact_dir.exists() {
std::fs::create_dir(artifact_dir)?;
fs_err::create_dir(artifact_dir)?;
}

let canister_path = PathBuf::from_str(&format!("{}.wasm", artifact_path.display())).map_err(|e| anyhow::anyhow!(e))?;
Expand All @@ -84,7 +84,7 @@ impl StepCtx {
let response = self.client.get(&url).send().await?.error_for_status()?.bytes().await?;
let mut d = GzDecoder::new(&response[..]);
let mut collector: Vec<u8> = vec![];
let mut file = std::fs::File::create(&canister_path)?;
let mut file = fs_err::File::create(&canister_path)?;
d.read_to_end(&mut collector)?;

file.write_all(&collector)?;
Expand All @@ -95,13 +95,13 @@ impl StepCtx {
pub async fn download_executable(&self, executable: &str, version: &str) -> anyhow::Result<PathBuf> {
let cache = dirs::cache_dir().ok_or(anyhow::anyhow!("Can't cache dir"))?.join(IC_EXECUTABLES_DIR);
if !cache.exists() {
std::fs::create_dir_all(&cache)?;
fs_err::create_dir_all(&cache)?;
}

let exe_path = cache.join(format!("{}/{}.{}", executable, executable, version));
let artifact_dir = exe_path.parent().unwrap();
if !artifact_dir.exists() {
std::fs::create_dir(artifact_dir)?;
fs_err::create_dir(artifact_dir)?;
}

if exe_path.exists() && exe_path.is_file() {
Expand All @@ -128,7 +128,7 @@ impl StepCtx {
let response = self.client.get(&url).send().await?.error_for_status()?.bytes().await?;
let mut d = GzDecoder::new(&response[..]);
let mut collector: Vec<u8> = vec![];
let mut file = std::fs::File::create(&exe_path)?;
let mut file = fs_err::File::create(&exe_path)?;
d.read_to_end(&mut collector)?;

file.write_all(&collector)?;
Expand Down
4 changes: 2 additions & 2 deletions rs/cli/src/store.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
use std::{io::Read, os::unix::fs::PermissionsExt, path::PathBuf, sync::Arc, time::Duration};

use flate2::bufread::GzDecoder;
use ic_canisters::governance::governance_canister_version;
use ic_management_backend::{
Expand All @@ -11,6 +9,8 @@ use ic_management_backend::{
use ic_management_types::Network;
use ic_registry_local_registry::LocalRegistry;
use log::{debug, info, warn};
use std::os::unix::fs::PermissionsExt;
use std::{io::Read, path::PathBuf, sync::Arc, time::Duration};

use crate::{
auth::Neuron,
Expand Down
6 changes: 3 additions & 3 deletions rs/cli/src/unit_tests/cordoned_feature_fetcher.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,15 +8,15 @@ use serde_yaml::Mapping;
use crate::store::Store;

fn ensure_empty(file: PathBuf) {
std::fs::write(file, "").unwrap();
fs_err::write(file, "").unwrap();
}

fn write_to_cache(contents: &[CordonedFeature], path: PathBuf) {
let mut mapping = Mapping::new();
mapping.insert("features".into(), serde_yaml::to_value(contents).unwrap());
let root = serde_yaml::Value::Mapping(mapping);

std::fs::write(path, serde_yaml::to_string(&root).unwrap()).unwrap()
fs_err::write(path, serde_yaml::to_string(&root).unwrap()).unwrap()
}

#[derive(Debug)]
Expand Down Expand Up @@ -122,7 +122,7 @@ fn cordoned_feature_fetcher_tests() {
}

let cordoned_features = maybe_cordoned_features.unwrap();
let cache_contents = std::fs::read_to_string(store.cordoned_features_file_outer().unwrap()).unwrap();
let cache_contents = fs_err::read_to_string(store.cordoned_features_file_outer().unwrap()).unwrap();
let cordoned_features_from_cache = cordoned_feature_fetcher.parse_outer(cache_contents.as_bytes()).unwrap();

if !cordoned_features.eq(&cordoned_features_from_cache) {
Expand Down
2 changes: 1 addition & 1 deletion rs/cli/src/unit_tests/ctx_init.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ fn status_file_path() -> PathBuf {
fn get_deleted_status_file() -> PathBuf {
let status_file = status_file_path();
if status_file.exists() {
std::fs::remove_file(&status_file).unwrap()
fs_err::remove_file(&status_file).unwrap()
}
status_file
}
Expand Down
6 changes: 3 additions & 3 deletions rs/cli/src/unit_tests/health_client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -101,8 +101,8 @@ fn health_client_tests() {
let cache_path = store.node_health_file_outer(&network).unwrap();

match &scenario.cache {
Some(cache) => std::fs::write(&cache_path, serde_json::to_string_pretty(cache).unwrap()).unwrap(),
None => std::fs::write(&cache_path, "").unwrap(),
Some(cache) => fs_err::write(&cache_path, serde_json::to_string_pretty(cache).unwrap()).unwrap(),
None => fs_err::write(&cache_path, "").unwrap(),
}

let health_client = store.health_client(&network).unwrap();
Expand All @@ -122,7 +122,7 @@ fn health_client_tests() {
}

let nodes_info = maybe_nodes_info.unwrap();
let contents = std::fs::read_to_string(&cache_path).unwrap();
let contents = fs_err::read_to_string(&cache_path).unwrap();
let cached: Vec<ShortNodeInfo> = serde_json::from_str(&contents).unwrap();

if !PartialEq::eq(&cached, &nodes_info) {
Expand Down
6 changes: 3 additions & 3 deletions rs/cli/src/unit_tests/node_labels.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ use itertools::Itertools;
use crate::store::Store;

fn ensure_empty(path: PathBuf) {
std::fs::write(path, "").unwrap()
fs_err::write(path, "").unwrap()
}

fn write_cache(guests: &[Guest], path: PathBuf) {
Expand All @@ -24,7 +24,7 @@ fn write_cache(guests: &[Guest], path: PathBuf) {
let mut root = serde_yaml::Mapping::new();
root.insert("data".into(), data.into());

std::fs::write(path, serde_yaml::to_string(&root).unwrap()).unwrap()
fs_err::write(path, serde_yaml::to_string(&root).unwrap()).unwrap()
}

#[derive(Debug)]
Expand Down Expand Up @@ -146,7 +146,7 @@ fn test_node_labels() {
}

let labels = maybe_labels.unwrap();
let content = std::fs::read_to_string(labels_path.clone()).unwrap();
let content = fs_err::read_to_string(labels_path.clone()).unwrap();
let labels_from_cache = node_labels::parse_data(content).unwrap();
if !labels.eq(&labels_from_cache) {
failed_scenarios.push((labels, scenario));
Expand Down
Loading

0 comments on commit 2291c60

Please sign in to comment.