fix(commands): Skip warmup script on --dry-run

[SomeThink1729]

This resolves #1428 and also skips it on restore On prune and restore --dry-run would still issue the warmup script when using hot/cold storage

Right now this doesn't add any information that the warmup script WOULD be executed, maybe it'd be a good information

I haven't added tests yet, but first I wanted to make sure #1428 is unintended behavior and should be fixed

New output [INFO] using config /home/username/.config/rustic/warm_up.toml [INFO] using warm-up command ./warmup.sh '%id' enter repository password: [hidden] [INFO] repository local:/some/test_repos/cold#local:/some/test_repos/hot: password is correct. [INFO] using cache at home/username/.cache/rustic/3b30726615c501982a75710ca6da0bf48bdd0aba5f40d93f0fc9762689e36098 [00:00:00] reading index... ████████████████████████████████████████ 0/0 [00:00:00] reading snapshots... ████████████████████████████████████████ 0/0 [00:00:00] finding used blobs... ████████████████████████████████████████ 0/0 [00:00:00] getting packs from repository... [INFO] to repack: 0 packs, 0 blobs, 0 B [INFO] this removes: 0 blobs, 0 B [INFO] to delete: 0 packs, 0 blobs, 0 B [INFO] total prune: 0 blobs, 0 B [INFO] remaining: 0 blobs, 0 B [INFO] unused size after prune: 0 B (NaN% of remaining size) [INFO] packs marked for deletion: 0, 0 B [INFO] - complete deletion: 0, 0 B [INFO] - keep marked: 0, 0 B [INFO] - recover: 0, 0 B

Old output (doesn't show lines that are equal) ... [INFO] - recover: 0, 0 B [00:00:00] warming up packs... ████████████████████████████████████████ 0/0

[aawsome]

@SomeThink1729 Thanks a lot for opening this PR!

I think we should first talk a bit about the general direction. While I see that users may not want to warm-up during a --dry-run, there are also others which exactly do want to do this - run a dry-run to warm-up some data which can then be processed in future.

So, I suggest that we make warming up in dry run an option to choose, i.e. either add a --dry-run-warm-up option along with --dry-run or add a --dry-run-no-warm-up (names can be discussed) option to keep the current behavior but still allow users to choose.

@SomeThink1729 Which variant do you like more? Can you add this to this PR?

[SomeThink1729]

@aawsome Thanks for the feedback.

Until now I didn't see the use case you're describing, but now it totally makes sense. I only saw the warm-up script as something to establish a connection - login somewhere else etc. transform data, which you wan't to skip if you --dry-run.

For me personally I think this use case is more naturally and I'd put in under --dry-run and the one you're describing under --dry-run-warmup, but this would lead to a breaking change in --dry-run, which I don't really like.

So I'll implement it the other way round (--dry-run and --dry-run-no-warmup) and we can discuss things further, what you like more etc. I think changing the names is not that much of an effort after adding the feature.

[aawsome]

After thinking about it, I agree that it may make more sense to have dry run not warm up by default. The next release will anyway break smaller things, so I think we can also introduce a breaking change here...

[SomeThink1729]

@aawsome I've now added the discussed behaviour.

--dry-run and --dry-run-warmup clash with each other, since one should issue the script and the other doesn't. Right now it takes the stricter one of both, so it skips warmup, but I prefer to do nothing and return an error that this config is invalid. I thought about adding something like this:

pub fn validate(&self) -> Result<(), SomeKindOfError> {
// do some checks
}

to check if both arguments are supplied or if the config is valid and continue execution. However I have no clue what kind of error type I would need or where to call this function.

Do you think it's necessary and would like to include such a check or do you think its unnecessary? Tests are still missing, but I intend to write some before the merge

[aawsome]

--dry-run and --dry-run-warmup clash with each other, since one should issue the script and the other doesn't. Right now it takes the stricter one of both, so it skips warmup, but I prefer to do nothing and return an error that this config is invalid.

We could let clap handle this. However, this is also present in the config file, so it won't work when given there... As we use anyhow for the rustic CLI, that would mean to just return an anyhow! error in this case or just use bail. IIRC in the backup command there are some checks which are handled in this way.

An alternative would be to have a --dry-run option and the --dry-run-warmup be an option that only yields in combination with --dry-run. So, --dry-run would not warmup and --dry-run --dry-run-warmup would warmup. But using only --dry-run-warmup would do nothing or only give a warning that it is ignored as no dry-run is given... This however would mean to change the logic a bit...

[aawsome]

@SomeThink1729 would you like to continue working on this? If not, I can also finish this to merge it soon...