Merge pull request #72 from bartfeenstra/subprocess-exceptions-2

Handle subprocess exceptions for the rsync transfers.

backuppy/task.py

@@ -1,14 +1,16 @@
 """Code to run back-ups."""
 import subprocess
-
 import os
 
 from backuppy.config import Configuration
 from backuppy.location import new_snapshot_name, Path, FilePath, DirectoryPath
 
 
 def rsync(configuration, origin, destination, path=None):
-    """Invoke rsync."""
+    """Invoke rsync.
+
+    :raise: subprocess.CalledProcessError
+    """
     args = ['rsync', '-ar', '--numeric-ids']
 
     ssh_options = configuration.ssh_options()
@@ -31,9 +33,7 @@ def rsync(configuration, origin, destination, path=None):
     else:
         args.append(destination.to_rsync(path))
 
-    exit_code = subprocess.check_call(args)
-
-    return exit_code == 0
+    subprocess.check_call(args)
 
 
 def backup(configuration, path=None):
@@ -63,14 +63,14 @@ def backup(configuration, path=None):
     snapshot_name = new_snapshot_name()
     target.snapshot(snapshot_name)
 
-    result = rsync(configuration, source, target, path)
-
-    if result:
+    try:
+        rsync(configuration, source, target, path)
         notifier.confirm('Back-up %s complete.' % configuration.name)
-    else:
+        return True
+    except subprocess.CalledProcessError:
+        configuration.logger.exception('An rsync error occurred.')
         notifier.confirm('Back-up %s failed.' % configuration.name)
-
-    return result
+        return False
 
 
 def restore(configuration, path=None):
@@ -98,11 +98,11 @@ def restore(configuration, path=None):
 
     notifier.inform('Restoring %s...' % configuration.name)
 
-    result = rsync(configuration, target, source, path)
-
-    if result:
+    try:
+        rsync(configuration, target, source, path)
         notifier.confirm('Restoration of back-up %s complete.' % configuration.name)
-    else:
+        return True
+    except subprocess.CalledProcessError:
+        configuration.logger.exception('An rsync error occurred.')
         notifier.alert('Restoration of back-up %s failed.' % configuration.name)
-
-    return result
+        return False

backuppy/tests/test_task.py

@@ -10,9 +10,9 @@
 from backuppy.tests import assert_paths_identical, build_files_stage_1, build_files_stage_2
 
 try:
-    from unittest.mock import Mock
+    from unittest.mock import Mock, patch
 except ImportError:
-    from mock import Mock
+    from mock import Mock, patch
 
 try:
     from tempfile import TemporaryDirectory
@@ -204,6 +204,23 @@ def test_backup_with_unavailable_target(self):
                 result = backup(configuration)
                 self.assertFalse(result)
 
+    @patch('backuppy.task.rsync')
+    def test_backup_with_subprocess_error(self, m):
+        m.side_effect = subprocess.CalledProcessError(7, '')
+        # Create the source directory.
+        with TemporaryDirectory() as source_path:
+            # Create the target directory.
+            with TemporaryDirectory() as target_path:
+                configuration = Configuration('Foo', verbose=True)
+                configuration.notifier = Mock(Notifier)
+                configuration.source = PathSource(
+                    configuration.logger, configuration.notifier, source_path + '/')
+                configuration.target = PathTarget(
+                    configuration.logger, configuration.notifier, target_path)
+
+                result = backup(configuration)
+                self.assertFalse(result)
+
 
 class RestoreTest(TestCase):
     def test_restore_all(self):
@@ -311,3 +328,20 @@ def test_restore_with_unavailable_target(self):
                     configuration.logger, configuration.notifier, target_path + '/NonExistentPath')
                 result = restore(configuration)
                 self.assertFalse(result)
+
+    @patch('backuppy.task.rsync')
+    def test_restore_with_subprocess_error(self, m):
+        m.side_effect = subprocess.CalledProcessError(7, '')
+        # Create the target directory.
+        with TemporaryDirectory() as target_path:
+            # Create the source directory.
+            with TemporaryDirectory() as source_path:
+                configuration = Configuration('Foo', verbose=True)
+                configuration.notifier = Mock(Notifier)
+                configuration.source = PathSource(
+                    configuration.logger, configuration.notifier, source_path + '/')
+                configuration.target = PathTarget(
+                    configuration.logger, configuration.notifier, target_path)
+
+                result = restore(configuration)
+                self.assertFalse(result)