Skip to content
This repository was archived by the owner on Nov 15, 2023. It is now read-only.
Merged
Changes from 1 commit
Commits
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
Prev Previous commit
Whitespace
  • Loading branch information
gavofyork authored Jul 25, 2018
commit 3dc3838d7e65420b166d27c189df9ee0cb434c07
18 changes: 8 additions & 10 deletions substrate/network-libp2p/src/network_state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -176,21 +176,18 @@ impl NetworkState {
let node_store = if let Some(ref path) = config.net_config_path {
let path = Path::new(path).join(NODES_FILE);
if let Ok(node_store) = JsonPeerstore::new(path.clone()) {
debug!(target: "sub-libp2p", "Initialized peer store for JSON \
file {:?}", path);
debug!(target: "sub-libp2p", "Initialized peer store for JSON file {:?}", path);
NodeStore::Json(node_store)
} else {
warn!(target: "sub-libp2p", "Failed to open peer storage {:?}\
; peers file will be reset", path);
warn!(target: "sub-libp2p", "Failed to open peer storage {:?}; peers file will be reset", path);
fs::remove_file(&path).expect("Failed deleting peers.json");

// we check for about 1s if the file was really deleted and move on
for _x in 0..200 {
if !Path::new(&path).exists() {
break;
} else {
debug!("Waiting for effective deletion of invalid/outdate \
peers.json");
debug!("Waiting for effective deletion of invalid/outdate peers.json");
thread::sleep(time::Duration::from_millis(5));
}
}
Expand All @@ -199,14 +196,15 @@ impl NetworkState {
debug!("peers.json reset");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note that this also happens if the file doesn't exist in the first place, ie. if the user starts polkadot for the first time, so the message is not totally correct.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the file does not exist in the first place, the first test L179 will kick in and the message you mention L196 will never show up.

NodeStore::Json(peerstore)
} else {
warn!(target: "sub-libp2p", "Failed to reset peer storage\
{:?}; peers change will not be saved", path);
warn!(target: "sub-libp2p",
"Failed to reset peer storage {:?}; peers change will not be saved",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This can also happen if the user doesn't have the permission to open the file, so the message should just be Failed to open peer storage IMO.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would not the message L182 already say that?
I can change the message, no problem. We would then have 2 identical messages. Would you prefer that?

If the file cannot be open because of permissions, this should be caught by the first tests. The message Failed to reset peer storage really comes second, only after we tried deleting and recreating the file.

path
);
NodeStore::Memory(MemoryPeerstore::empty())
}
}
} else {
debug!(target: "sub-libp2p", "No peers file configured ; peers \
won't be saved");
debug!(target: "sub-libp2p", "No peers file configured ; peers won't be saved");
NodeStore::Memory(MemoryPeerstore::empty())
};

Expand Down