fix(commands): return error if check fails #224
Conversation
aawsome
changed the title
Codecov ReportAttention: Patch coverage is
Additional details and impacted files
|
|
I'll look through it tomorrow. 👍🏽 |
| pub enum CheckErrorLevel { | ||
| /// A warning: Something is strange and should be corrected, but repository integrity is not affected. | ||
| Warn, | ||
| /// An erro: Something in the repository is not as it should be. |
There was a problem hiding this comment.
| /// An erro: Something in the repository is not as it should be. | |
| /// An error: Something in the repository is not as it should be. |
| #[derive(Debug)] | ||
| /// `CheckResults` is a list of errors encountered during the check. | ||
| pub struct CheckResults { | ||
| /// The list of errors with severity level. | ||
| pub errors: Vec<(CheckErrorLevel, CheckError)>, | ||
| } |
There was a problem hiding this comment.
Wondering if we could replace these with: https://crates.io/crates/lazy_errors
Didn't use it, but it sounded interesting on Reddit last week: https://www.reddit.com/r/rust/comments/1ddi1nb/i_published_my_first_crate_lazy_errors/
| #[derive(Debug, PartialEq, Eq)] | ||
| #[non_exhaustive] | ||
| /// `CheckErrorLevel` describes severity levels of problems identified by check. | ||
| pub enum CheckErrorLevel { |
There was a problem hiding this comment.
I would probably stick to the kind naming scheme we have in rustic_core, so something like CheckErrorKind.
| pub(crate) fn run<P: ProgressBars, S: Open>(self, repo: &Repository<P, S>) -> RusticResult<()> { | ||
| pub(crate) fn check<P: ProgressBars, S: Open>( | ||
| &self, | ||
| opts: CheckOptions, |
There was a problem hiding this comment.
I like that much better, it's a good usage of structs, to pass these inside like this, so it's also easy to impl Default on CheckOptions 👍🏽
| // TODO: Make concurrency (20) customizable | ||
| check_cache_files(20, cache, raw_be, file_type, &p)?; | ||
| self.check_cache_files(20, cache, raw_be, file_type, &p)?; |
There was a problem hiding this comment.
Not sure, if that could be just a field in CheckOptions? or maybe another struct ParallelismOptions composed into CheckOptions that we can use in rustic_core.
| // TODO: Make concurrency (5) customizable | ||
| check_cache_files(5, cache, raw_be, FileType::Pack, &p)?; | ||
| self.check_cache_files(5, cache, raw_be, FileType::Pack, &p)?; |
There was a problem hiding this comment.
Same here, maybe ParallelismOptions could be useful here as well?
| Err(err) => error!("Error reading pack {id} : {err}",), | ||
| Err(err) => self.add_error(CheckError::ErrorReadingPack { | ||
| id, | ||
| err: Box::new(err), |
There was a problem hiding this comment.
would call it source so it's more clear
| } | ||
| self.errors | ||
| .lock() | ||
| .unwrap() |
There was a problem hiding this comment.
would at least prefer an expect here
closes #222