duplicity-team team mailing list archive

[Michael Terry <michael.terry@xxxxxxxxxxxxx>]

Michael Terry has proposed merging lp:~mterry/duplicity/retry-decorator into lp:duplicity with lp:~mterry/duplicity/gio-name as a prerequisite.

Requested reviews:
  duplicity-team (duplicity-team)
Related bugs:
  Bug #545486 in Duplicity: "Backup fails with 'Error: No data available'"
  https://bugs.launchpad.net/duplicity/+bug/545486

For more details, see:
https://code.launchpad.net/~mterry/duplicity/retry-decorator/+merge/63571

This branch adds a decorator to duplicity.backend for use by the backends if they like.  Decorators were added in Python 2.4, so I believe it's safe to rely on.

I then started using the decorator in the giobackend.  And finally (the real purpose of all this), I added the decorator to giobackend's list and delete operations.

This was motivated by bug 545486 where the user was getting the message "No data available" seemingly because there was a long gap between mounting the remote location and doing the first operation (list) due to scanning during a dry-run.  The first list operation would fail, but if we did a second one, it would succeed.

Presumably GIO was reconnecting but still failing that first time.  Seems like bad GIO design, but this works around it anyway.  Plus, better to retry than not whenever we hit the backend.  Some of the same issues with put/get can be had with list/delete.

I haven't converted any of the other backends to use the decorator, but it seems like it could reduce a bit of code.
-- 
https://code.launchpad.net/~mterry/duplicity/retry-decorator/+merge/63571
Your team duplicity-team is requested to review the proposed merge of lp:~mterry/duplicity/retry-decorator into lp:duplicity.

=== modified file 'duplicity/backend.py'
--- duplicity/backend.py	2011-04-04 15:50:11 +0000
+++ duplicity/backend.py	2011-06-06 15:51:07 +0000
@@ -293,6 +293,22 @@
     return parsed_url.geturl().replace(parsed_url.netloc, straight_netloc, 1)
 
 
+# Decorator for backend operation functions to simplify writing one that
+# retries.  Make sure to add a keyword argument 'raise_errors' to your function
+# and if it is true, raise an exception on an error.  If false, fatal-log it.
+def retry(fn):
+    def iterate(*args):
+        for n in range(1, globals.num_retries):
+            try:
+                return fn(*args, raise_errors=True)
+            except:
+                # actual error doesn't matter, fn will have logged it
+                pass
+        # Now try one last time, but fatal-log instead of raising errors
+        return fn(*args, raise_errors=False)
+    return iterate
+
+
 class Backend:
     """
     Represents a generic duplicity backend, capable of storing and

=== modified file 'duplicity/backends/giobackend.py'
--- duplicity/backends/giobackend.py	2011-06-06 15:51:07 +0000
+++ duplicity/backends/giobackend.py	2011-06-06 15:51:07 +0000
@@ -27,6 +27,7 @@
 import glib #@UnresolvedImport
 
 import duplicity.backend
+from duplicity.backend import retry
 from duplicity import log
 from duplicity import globals
 from duplicity import util
@@ -89,7 +90,9 @@
                                % str(e), log.ErrorCode.connection_failed)
         loop.quit()
 
-    def handle_error(self, e, op, file1=None, file2=None):
+    def handle_error(self, raise_error, e, op, file1=None, file2=None):
+        if raise_error:
+            raise e
         code = log.ErrorCode.backend_error
         if isinstance(e, gio.Error):
             if e.code == gio.ERROR_PERMISSION_DENIED:
@@ -105,21 +108,19 @@
     def copy_progress(self, *args, **kwargs):
         pass
 
-    def copy_file(self, op, source, target):
-        exc = None
-        for n in range(1, globals.num_retries+1):
-            log.Info(_("Writing %s") % target.get_parse_name())
-            try:
-                source.copy(target, self.copy_progress,
-                            gio.FILE_COPY_OVERWRITE | gio.FILE_COPY_NOFOLLOW_SYMLINKS)
-                return
-            except Exception, e:
-                log.Warn("Write of '%s' failed (attempt %s): %s: %s"
-                        % (target.get_parse_name(), n, e.__class__.__name__, str(e)))
-                log.Debug("Backtrace of previous error: %s"
-                          % exception_traceback())
-                exc = e
-        self.handle_error(exc, op, source.get_parse_name(), target.get_parse_name())
+    @retry
+    def copy_file(self, op, source, target, raise_errors=False):
+        log.Info(_("Writing %s") % target.get_parse_name())
+        try:
+            source.copy(target, self.copy_progress,
+                        gio.FILE_COPY_OVERWRITE | gio.FILE_COPY_NOFOLLOW_SYMLINKS)
+        except Exception, e:
+            log.Warn("Write of '%s' failed (attempt %s): %s: %s"
+                    % (target.get_parse_name(), n, e.__class__.__name__, str(e)))
+            log.Debug("Backtrace of previous error: %s"
+                      % exception_traceback())
+            self.handle_error(raise_errors, e, op, source.get_parse_name(),
+                              target.get_parse_name())
 
     def put(self, source_path, remote_filename = None):
         """Copy file to remote"""
@@ -136,28 +137,34 @@
         self.copy_file('get', source_file, target_file)
         local_path.setdata()
 
-    def list(self):
+    @retry
+    def list(self, raise_errors=False):
         """List files in that directory"""
+        files = []
         try:
             enum = self.remote_file.enumerate_children(gio.FILE_ATTRIBUTE_STANDARD_NAME,
                                                        gio.FILE_QUERY_INFO_NOFOLLOW_SYMLINKS)
-        except Exception, e:
-            self.handle_error(e, 'list', self.remote_file.get_parse_name())
-        files = []
-        try:
             info = enum.next_file()
             while info:
                 files.append(info.get_name())
                 info = enum.next_file()
-            return files
         except Exception, e:
-            self.handle_error(e, 'list')
+            self.handle_error(raise_errors, e, 'list',
+                              self.remote_file.get_parse_name())
+        return files
 
-    def delete(self, filename_list):
+    @retry
+    def delete(self, filename_list, raise_errors=False):
         """Delete all files in filename list"""
         assert type(filename_list) is not types.StringType
-        try:
-                for filename in filename_list:
-                        self.remote_file.get_child(filename).delete()
-        except Exception, e:
-            self.handle_error(e, 'delete', self.remote_file.get_child(filename).get_parse_name())
+        for filename in filename_list:
+            target_file = self.remote_file.get_child(filename)
+            try:
+                target_file.delete()
+            except Exception, e:
+                if isinstance(e, gio.Error):
+                    if e.code == gio.ERROR_NOT_FOUND:
+                        continue
+                self.handle_error(raise_errors, e, 'delete',
+                                  target_file.get_parse_name())
+                return