Added --keep-all, as an alias for '--keep-last <inf>', as an option…

[Michael-Girma]

… for borg-prune. Fixes #6656

• Added a subparser argument that stores a large value for --keep-last when pruning.

[codecov-commenter]

Codecov Report

Merging #7462 (0301451) into master (1e1c922) will increase coverage by 0.00%.
The diff coverage is 100.00%.

📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

@@           Coverage Diff           @@
##           master    #7462   +/-   ##
=======================================
  Coverage   83.87%   83.87%           
=======================================
  Files          67       67           
  Lines       11817    11818    +1     
  Branches     2155     2155           
=======================================
+ Hits         9911     9912    +1     
  Misses       1332     1332           
  Partials      574      574           

Impacted Files	Coverage Δ
src/borg/archiver/prune_cmd.py	87.14% <100.00%> (+0.09%)	⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

[ThomasWaldmann]

Hehe, looks good. Considering the special infinity value, I guess a test for this would be good (just test the code does not crash and keeps all archives present, except old checkpoint archives).

[jdchristensen]

What happens if someone does --keep-all --keep-secondly=10? Does the 10 overwrite the infinite value? This might be surprising to the user since the options seem independent. (This is already an issue with --keep-last 100 --keep-secondly 10 as well, I believe.) Maybe --keep-last should be implemented separately from --keep-secondly? (I'd also be inclined to remove --keep-secondly in borg2.) (I'm not worried about the interaction between --keep-all and --keep-last, since --keep-all is documented to be equivalent to --keep-last <inf>.)

The above conflicts might happen because someone has a script that already has various --keep options, and adds --keep-all at the front, thinking that this would cause everything to be kept.

[Michael-Girma]

@jdchristensen, interesting insight. I'll take a look at the interactions between those flags. @ThomasWaldmann I did test that eye-catching infinite value locally (didn't test deeply, just created a few archives to see that it didn't crash). I can write up a unit test to see how well it works with keeping archives and removing checkpoints. I would love to hear your thoughts on @jdchristensen's remarks though.

[ThomasWaldmann]

Good catch! Issue #6269 applies here also, I guess.

[ThomasWaldmann]

If we have different argparse options, we could also use this:

https://docs.python.org/3/library/argparse.html#mutual-exclusion

[ThomasWaldmann]

I resolved the merge conflict and also had another look:

• Highlander is not needed for --keep-all - it can be given multiple times without causing an issue (because it is always using the same infinity value)
• but we should have the mutual exclusion for --keep-last, --keep-secondly, --keep-all.

@Michael-Girma Can you try to implement that and add a test?

[ThomasWaldmann]

@Michael-Girma did you see my last comment, can you fix it?

[Michael-Girma]

@ThomasWaldmann just seeing this. Will take a look tonight.

[ThomasWaldmann]

ping.

[ThomasWaldmann]

ping

[ThomasWaldmann]

ping

[ThomasWaldmann]

@Michael-Girma do you need help finishing this?