Restic fixes: pruning, process substitution (take 2) #78886
Conversation
| else "--files-from ${filesFromTmpFile}"; | ||
| pruneCmd = optional (builtins.length backup.pruneOpts > 0) [ | ||
| ( resticCmd + " forget --prune " + (concatStringsSep " " backup.pruneOpts) ) | ||
| ( resticCmd + " check" ) |
There was a problem hiding this comment.
I'm not 100% sure but I think check is not needed when running prune.
Check https://forum.restic.net/t/should-i-run-restic-check/1227
but it seems some people might want to run check --read-data from time to time.
There was a problem hiding this comment.
Also the prune command doesn't run because the check one is on the same line in the final systemd service file:
There was a problem hiding this comment.
Yeah, I'm seeing that missing newline too. I don't understand why, but I see it.
There was a problem hiding this comment.
Also I don't think I'm quite ready to rely on the accepted solution in that forum thread, because of the quote from the docs later on: https://forum.restic.net/t/should-i-run-restic-check/1227/9
There was a problem hiding this comment.
found the problem with the missing newline, see jerith666@dc6616b
There was a problem hiding this comment.
Also I don't think I'm quite ready to rely on the accepted solution in that forum thread, because of the quote from the docs later on: https://forum.restic.net/t/should-i-run-restic-check/1227/9
Yeah that's weird. fd0 is the main dev so we are supposed to be able to trust him. But he didn't bother to reply on the last comment.
Maybe we should ask him. It would be a shame to run an expansive operation like check daily until the next release.
There was a problem hiding this comment.
My test passes with jerith666@dc6616b .
Do you think it would be a good idea to make running restic check optional for now? And figure out the true best practices from the upstream dev later.
EDIT: check is already optional. If I think it's running to often I can disable pruning and do it myself.
|
I have a nixos test for the prune part: bbigras@a772ab6 Feel free to include it in this PR, otherwise I'll try to get it merged later. |
|
This pull request has been mentioned on NixOS Discourse. There might be relevant details there: https://discourse.nixos.org/t/nixos-20-03-feature-freeze/5655/8 |
|
Can you add jerith666@dc6616b to this PR please? |
|
@GrahamcOfBorg test restic |
|
@GrahamcOfBorg test restic |
|
@GrahamcOfBorg test restic |
|
thanks @Mic92 ❤️ |
|
This pull request has been mentioned on NixOS Discourse. There might be relevant details there: |
Motivation for this change
I rebased #44203 and replace some if expressions with optionals/optionalString/optionalAttr.
The result is untested.
cc @jerith666 @bbigras
Things done
sandboxinnix.confon non-NixOS linux)nix-shell -p nixpkgs-review --run "nixpkgs-review wip"./result/bin/)nix path-info -Sbefore and after)