nixos/duplicity: enable to prevent backup from growing infinitely #120622
Conversation
Current module add backups forever, with no way to prune old ones. Add an option to remove backups after n full backups or after some amount of time. Also run duplicity cleanup to clean unused files in case some previous backup was improperly interrupted.
ofborg
bot
added
10.rebuild-darwin: 0
10.rebuild-linux: 1-10
10.rebuild-linux: 1
labels
| in | ||
| '' | ||
| set -x | ||
| ${dup} cleanup ${target} --force ${extra} |
There was a problem hiding this comment.
Will all extra parameters that can be passed to 'duplicity incr' fit 'duplicity cleanup', remove-older-than, or remove-all-but-n-full?
There was a problem hiding this comment.
Yes. If you look at the man page you will see that all options are accepted by all verbs. For example, during testing I am using extraFlags = [ "--full-if-older-than" "7D" ];. This option does not make sense for cleanup, but it still works (it is ignored).
There was a problem hiding this comment.
In this case, instead of maxAge and maxFull, what do you think of an option to set the list of actions, the default being ["incr"].
There was a problem hiding this comment.
verbs don't take the same positional arguments: cleanup only takes the target, and incr takes both a source and a target, so there seems to be no way to provide such a unified interface.
There was a problem hiding this comment.
OK. We could add the source only for verbs that need it, but on the other hand the only verbs that make sense in a timer are:
- full
- incr
- remove-older-than
- remove-all-but-n-full
- remove-all-inc-of-but-n-full
so if you add an option for remove-all-inc-of-but-n-full we're covered.
(full is incr --full-if-older-than 0, probably.)
What do you think?
There was a problem hiding this comment.
I'd rather add options maxIncAge for remove-all-inc-but-n-full and mode = "full" | "incremental" to choose between incremental and full backups (and maybe an option to set --full-if-older-than).
This way you specify the end result you want instead of the list of actions to take. Also it's kind of self documenting by reading the option list.
There was a problem hiding this comment.
I just did that, please have a look.
| script = | ||
| let | ||
| target = escapeShellArg cfg.targetUrl; | ||
| extra = escapeShellArgs ([ "--archive-dir" stateDirectory ] ++ cfg.extraFlags); |
There was a problem hiding this comment.
Maybe rename 'extra' into 'flags'?
There was a problem hiding this comment.
I think we enter bikeshedding territory here.
There was a problem hiding this comment.
True. Feel free to ignore, I don't decide.
But it's confusing between extra an extraFlags. It's not extra, it's all flags. Or options.
|
The request looks fine. I haven't tested it. |
Do you intend to ? I have been using it for a week and it work, so I'm inclined to merge, but if you want to test I can wait :) |
| If <literal>"never"</literal> (the default) always do incremental | ||
| backups (the first backup will be a full backup, of course). If | ||
| <literal>"always"</literal> always do full backups (the first backup | ||
| will be a full backup, of course). Otherwise, this must be a string |
There was a problem hiding this comment.
"the first backup will be a full backup, of course" this doesn't really make sense for the "always" option.
| in | ||
| '' | ||
| set -x | ||
| ${dup} cleanup ${target} --force ${extra} |
Sorry, I don't have time right now. |
except restore ;)
symphorien
force-pushed
the
duplicity-master
branch
from
8464ffd
to
41c7fa4
Compare
|
Thanks for you review! I think I addressed it, I'll merge soon unless someone speaks up. |
In #120622 cleanup options were added, but `remove-all-inc-of-but-n-full` was misspelled and as such was not functioning.
Motivation for this change
In the current state, the module stores backup indefinitely, without a way to prune old backups. This adds new options to curb space usage.
First commit formats the file, second commit contains the logic.
Tested backported to 20.09.
cc @OlivierMarty who wrote the module, cc @peti as maintainer of duplicity
Things done
sandboxinnix.confon non-NixOS linux)nix-shell -p nixpkgs-review --run "nixpkgs-review wip"./result/bin/)nix path-info -Sbefore and after)