Generalize --use-rmdir to --removal-command, add tests (#11)

Author: xolox

README.rst

@@ -211,7 +211,14 @@ intended you have no right to complain ;-).
    remote system over SSH)."
    "``-n``, ``--dry-run``","Don't make any changes, just print what would be done. This makes it easy
    to evaluate the impact of a rotation scheme without losing any backups."
-   "``-D``, ``--use-rmdir``","Use ""rmdir"" to remove the backups (useful with CephFS snapshots for example)."
+   "``-C``, ``--removal-command=CMD``","Change the command used to remove backups. The value of ``CMD`` defaults to
+   ``rm ``-f``R``. This choice was made because it works regardless of whether
+   ""backups to be rotated"" are files or directories or a mixture of both.
+   
+   As an example of why you might want to change this, CephFS snapshots are
+   represented as regular directory trees that can be deleted at once with a
+   single 'rmdir' command (even though according to POSIX semantics this
+   command should refuse to remove nonempty directories, but I digress)."
    "``-v``, ``--verbose``",Increase logging verbosity (can be repeated).
    "``-q``, ``--quiet``",Decrease logging verbosity (can be repeated).
    "``-h``, ``--help``",Show this message and exit.
@@ -360,8 +367,11 @@ Supported configuration options
   - If an include or exclude list is defined in the configuration file it
     overrides the include or exclude list given on the command line.
 
-- The ``prefer-recent``, ``strict``, ``rmdir`` and ``use-sudo`` options expect a
-  boolean value (``yes``, ``no``, ``true``, ``false``, ``1`` or ``0``).
+- The ``prefer-recent``, ``strict`` and ``use-sudo`` options expect a boolean
+  value (``yes``, ``no``, ``true``, ``false``, ``1`` or ``0``).
+
+- The ``removal-command`` option can be used to customize the command that is
+  used to remove backups.
 
 - The ``ionice`` option expects one of the I/O scheduling class names ``idle``,
   ``best-effort`` or ``realtime``.

rotate_backups/__init__.py

@@ -1,7 +1,7 @@
 # rotate-backups: Simple command line interface for backup rotation.
 #
 # Author: Peter Odding <peter@peterodding.com>
-# Last Change: April 27, 2018
+# Last Change: August 2, 2018
 # URL: https://github.com/xolox/python-rotate-backups
 
 """
@@ -19,6 +19,7 @@
 import numbers
 import os
 import re
+import shlex
 
 # External dependencies.
 from dateutil.relativedelta import relativedelta
@@ -197,8 +198,11 @@ def load_config_file(configuration_file=None, expand=True):
                        exclude_list=split(items.get('exclude-list', '')),
                        io_scheduling_class=items.get('ionice'),
                        strict=coerce_boolean(items.get('strict', 'yes')),
-                       prefer_recent=coerce_boolean(items.get('prefer-recent', 'no')),
-                       rmdir=items.get('use-rmdir'))
+                       prefer_recent=coerce_boolean(items.get('prefer-recent', 'no')))
+        # Don't override the value of the 'removal_command' property unless the
+        # 'removal-command' configuration file option has a value set.
+        if items.get('removal-command'):
+            options['removal_command'] = shlex.split(items['removal-command'])
         # Expand filename patterns?
         if expand and location.have_wildcards:
             logger.verbose("Expanding filename pattern %s on %s ..", location.directory, location.context)
@@ -240,11 +244,12 @@ def __init__(self, rotation_scheme, **options):
         Initialize a :class:`RotateBackups` object.
 
         :param rotation_scheme: Used to set :attr:`rotation_scheme`.
-        :param options: Any keyword arguments are used to set the values of the
-                        properties :attr:`config_file`, :attr:`dry_run`,
-                        :attr:`rmdir`, :attr:`exclude_list`,
-                        :attr:`include_list`, :attr:`io_scheduling_class` and
-                        :attr:`strict`.
+        :param options: Any keyword arguments are used to set the values of
+                        instance properties that support assignment
+                        (:attr:`config_file`, :attr:`dry_run`,
+                        :attr:`exclude_list`, :attr:`include_list`,
+                        :attr:`io_scheduling_class`, :attr:`removal_command`
+                        and :attr:`strict`).
         """
         options.update(rotation_scheme=rotation_scheme)
         super(RotateBackups, self).__init__(**options)
@@ -271,18 +276,6 @@ def dry_run(self):
         """
         return False
 
-    @mutable_property
-    def rmdir(self):
-        """
-        :data:`True` to use `rmdir` to remove the snapshots, :data:`False` to use `rm -r` (defaults to :data:`False`).
-
-        Normally the backups are removed one file at a file using the command `rm -r`.
-        Some file-systems are capable of removing a whole directory with the command `rmdir`,
-        even when the directory is not empty. For example, this is how CephFS snapshots are
-        removed.
-        """
-        return False
-
     @cached_property(writable=True)
     def exclude_list(self):
         """
@@ -339,6 +332,24 @@ def prefer_recent(self):
         """
         return False
 
+    @mutable_property
+    def removal_command(self):
+        """
+        The command used to remove backups (a list of strings).
+
+        By default the command ``rm -fR`` is used. This choice was made because
+        it works regardless of whether the user's "backups to be rotated" are
+        files or directories or a mixture of both.
+
+        .. versionadded: 5.3
+           This option was added as a generalization of the idea suggested in
+           `pull request 11`_, which made it clear to me that being able to
+           customize the removal command has its uses.
+
+        .. _pull request 11: https://github.com/xolox/python-rotate-backups/pull/11
+        """
+        return ['rm', '-fR']
+
     @required_property
     def rotation_scheme(self):
         """
@@ -481,18 +492,16 @@ class together to implement backup rotation with an easy to use Python
             else:
                 logger.info("Deleting %s ..", friendly_name)
                 if not self.dry_run:
-                    if not self.rmdir:
-                        command = location.context.prepare(
-                            'rm', '-Rf', backup.pathname,
-                            group_by=(location.ssh_alias, location.mount_point),
-                            ionice=self.io_scheduling_class,
-                        )
-                    else:
-                        command = location.context.prepare(
-                            'rmdir', backup.pathname,
-                            group_by=(location.ssh_alias, location.mount_point),
-                            ionice=self.io_scheduling_class,
-                        )
+                    # Copy the list with the (possibly user defined) removal command.
+                    removal_command = list(self.removal_command)
+                    # Add the pathname of the backup as the final argument.
+                    removal_command.append(backup.pathname)
+                    # Construct the command object.
+                    command = location.context.prepare(
+                        command=removal_command,
+                        group_by=(location.ssh_alias, location.mount_point),
+                        ionice=self.io_scheduling_class,
+                    )
                     rotation_commands.append(command)
                     if not prepare:
                         timer = Timer()

rotate_backups/cli.py

@@ -1,7 +1,7 @@
 # rotate-backups: Simple command line interface for backup rotation.
 #
 # Author: Peter Odding <peter@peterodding.com>
-# Last Change: April 27, 2018
+# Last Change: August 2, 2018
 # URL: https://github.com/xolox/python-rotate-backups
 
 """
@@ -154,11 +154,16 @@
     Don't make any changes, just print what would be done. This makes it easy
     to evaluate the impact of a rotation scheme without losing any backups.
 
-  -D, --use-rmdir
+  -C, --removal-command=CMD
 
-    Remove backups by calling `rmdir' to directly remove the whole directory
-    instead of deleting each of the files in it. This works only with special
-    directories such as CephFS snapshot.
+    Change the command used to remove backups. The value of CMD defaults to
+    ``rm -fR``. This choice was made because it works regardless of whether
+    "backups to be rotated" are files or directories or a mixture of both.
+
+    As an example of why you might want to change this, CephFS snapshots are
+    represented as regular directory trees that can be deleted at once with a
+    single 'rmdir' command (even though according to POSIX semantics this
+    command should refuse to remove nonempty directories, but I digress).
 
   -v, --verbose
 
@@ -175,6 +180,7 @@
 
 # Standard library modules.
 import getopt
+import shlex
 import sys
 
 # External dependencies.
@@ -208,11 +214,11 @@ def main():
     selected_locations = []
     # Parse the command line arguments.
     try:
-        options, arguments = getopt.getopt(sys.argv[1:], 'M:H:d:w:m:y:I:x:jpri:c:r:uDnvqh', [
+        options, arguments = getopt.getopt(sys.argv[1:], 'M:H:d:w:m:y:I:x:jpri:c:r:uC:nvqh', [
             'minutely=', 'hourly=', 'daily=', 'weekly=', 'monthly=', 'yearly=',
             'include=', 'exclude=', 'parallel', 'prefer-recent', 'relaxed',
-            'ionice=', 'config=', 'use-sudo', 'dry-run', 'use-rmdir', 'verbose', 'quiet',
-            'help',
+            'ionice=', 'config=', 'use-sudo', 'dry-run', 'removal-command=',
+            'verbose', 'quiet', 'help',
         ])
         for option, value in options:
             if option in ('-M', '--minutely'):
@@ -247,8 +253,10 @@ def main():
             elif option in ('-n', '--dry-run'):
                 logger.info("Performing a dry run (because of %s option) ..", option)
                 kw['dry_run'] = True
-            elif option in ('-D', '--use-rmdir'):
-                kw['rmdir'] = True
+            elif option in ('-C', '--removal-command'):
+                removal_command = shlex.split(value)
+                logger.info("Using custom removal command: %s", removal_command)
+                kw['removal_command'] = removal_command
             elif option in ('-v', '--verbose'):
                 coloredlogs.increase_verbosity()
             elif option in ('-q', '--quiet'):

rotate_backups/tests.py

@@ -1,12 +1,13 @@
 # Test suite for the `rotate-backups' Python package.
 #
 # Author: Peter Odding <peter@peterodding.com>
-# Last Change: April 27, 2018
+# Last Change: August 3, 2018
 # URL: https://github.com/xolox/python-rotate-backups
 
 """Test suite for the `rotate-backups` package."""
 
 # Standard library modules.
+import datetime
 import logging
 import os
 
@@ -369,6 +370,16 @@ def test_minutely_rotation(self):
             assert os.path.exists(os.path.join(root, 'backup-2016-01-10_21-30-00'))
             assert os.path.exists(os.path.join(root, 'backup-2016-01-10_21-45-00'))
 
+    def test_removal_command(self):
+        """Test that the removal command can be customized."""
+        with TemporaryDirectory(prefix='rotate-backups-', suffix='-test-suite') as root:
+            today = datetime.datetime.now()
+            for date in today, (today - datetime.timedelta(hours=24)):
+                os.mkdir(os.path.join(root, date.strftime('%Y-%m-%d')))
+            program = RotateBackups(removal_command=['rmdir'], rotation_scheme=dict(monthly='always'))
+            commands = program.rotate_backups(root, prepare=True)
+            assert any(cmd.command_line[0] == 'rmdir' for cmd in commands)
+
     def test_filename_patterns(self):
         """Test support for filename patterns in configuration files."""
         with TemporaryDirectory(prefix='rotate-backups-', suffix='-test-suite') as root: