Log the error returned from SqliteStore::new and fs::create_dir_all#893
Conversation
This matches the logging done when setting up a VSS store.
|
👋 Thanks for assigning @tnull as a reviewer! |
|
🔔 1st Reminder Hey @tnull! This PR has been waiting for your review. |
tnull
left a comment
tnull
left a comment
There was a problem hiding this comment.
Hmm, it' a bit unfortunate that we now doubly-instantiate the logger. The pattern is pre-existing for VSS, but if we do this, maybe we want to introduce a build_with_store_and_logger internal method that all other build_with methods (including build_with_store) use, to avoid having to doubly setting up the logger every time?
This internal method allows us to avoid instantiating the logger twice during the creation of a `Node`.
|
Thanks for the review, I added a commit to make this change, let me know if you would prefer to squash it with the previous one. |
| .map_err(|_| BuildError::KVStoreSetupFailed)?; | ||
| self.build_with_store(node_entropy, kv_store) | ||
| .map_err(|e| { | ||
| log_error!(logger, "Failed to setup Sqlite store: {}", e); |
There was a problem hiding this comment.
nit: here and below
| log_error!(logger, "Failed to setup Sqlite store: {}", e); | |
| log_error!(logger, "Failed to set up Sqlite store: {}", e); |
There was a problem hiding this comment.
I'll land this shortly to keep moving, but feel free to add a commit addressing the nit in #863.
| .map_err(|_| BuildError::KVStoreSetupFailed)?; | ||
| self.build_with_store(node_entropy, kv_store) | ||
| .map_err(|e| { | ||
| log_error!(logger, "Failed to setup Sqlite store: {}", e); |
There was a problem hiding this comment.
I'll land this shortly to keep moving, but feel free to add a commit addressing the nit in #863.
4 participants
This matches the logging done when setting up a VSS store.