[RFC][PATCH] ftp transport, dsilver's branch

[Matthieu Moy]

Matthieu Moy <Matthieu.Moy at imag.fr> writes:

> Hi,
>
> First, a reminder: Daniel Silverstone's ftp branch contains some fixes
> that have not been merged. In particular, it allows reusing
> connections instead of creating several ones in parallel[1].

Followup to myself, with another patch:

It tries to solve the problem of ftp server closing the connection
arbitrarily after some time. The patch works for me (I could push a
branch of 849 revisions with 6 temporary failures, but it did it in
one go), but it probably needs to be improved before getting into
upstream. Daniel, I suggest that you merge it into your branch first.

Hopefully, a branch containing this fix will be available here within
a few minutes (I'm going to bed, so let's see if my patch still works
while I'm sleeping):

  http://test.bzr.free.fr/branches/bzr.ftp/

The patch (against dsilver's branch) is below:

=== modified file 'bzrlib/transport/ftp.py'
--- bzrlib/transport/ftp.py     
+++ bzrlib/transport/ftp.py     
@@ -37,7 +37,7 @@
 from bzrlib.transport import Transport
 from bzrlib.errors import (TransportNotPossible, TransportError,
                            NoSuchFile, FileExists)
-from bzrlib.trace import mutter
+from bzrlib.trace import mutter, warning
 
 
 _FTP_cache = {}
@@ -174,7 +174,7 @@
             mutter("FTP has not: %s" % self._abspath(relpath))
             return False
 
-    def get(self, relpath, decode=False):
+    def get(self, relpath, decode=False, retries=0):
         """Get the file at the given relative path.
 
         :param relpath: The relative path to the file
@@ -190,9 +190,23 @@
             ret.seek(0)
             return ret
         except ftplib.error_perm, e:
-            raise NoSuchFile(self.abspath(relpath), extra=extra)
-
-    def put(self, relpath, fp, mode=None):
+            raise NoSuchFile(self.abspath(relpath))
+        except ftplib.error_temp, e:
+            if retries > 1:
+                raise e
+            else:
+                warning("FTP temporary error: %s. Retrying." % str(e))
+                self._FTP_instance = None
+                return self.get(relpath, decode, retries+1)
+        except EOFError, e:
+            if retries > 1:
+                raise e
+            else:
+                warning("FTP connection closed. Trying to reopen.")
+                self._FTP_instance = None
+                return self.get(relpath, decode, retries+1)
+
+    def put(self, relpath, fp, mode=None, retries=0):
         """Copy the file-like or string object into the location.
 
         :param relpath: Location to put the contents, relative to base.
@@ -208,11 +222,25 @@
             f.storbinary('STOR '+self._abspath(relpath), fp, 8192)
         except ftplib.error_perm, e:
             if "no such file" in str(e).lower():
-                raise NoSuchFile(msg="Error storing %s: %s"
-                                 % (self.abspath(relpath), str(e)),
-                                 orig_error=e)
+                raise NoSuchFile("Error storing %s: %s"
+                                 % (self.abspath(relpath), str(e)))
             else:
                 raise FtpTransportError(orig_error=e)
+        except ftplib.error_temp, e:
+            if retries > 1:
+                raise e
+            else:
+                warning("FTP temporary error: %s. Retrying." % str(e))
+                self._FTP_instance = None
+                self.put(relpath, fp, mode, retries+1)
+        except EOFError, e:
+            if retries > 1:
+                raise e
+            else:
+                warning("FTP connection closed. Trying to reopen.")
+                self._FTP_instance = None
+                self.put(relpath, fp, mode, retries+1)
+
 
     def mkdir(self, relpath, mode=None):
         """Create a directory at the given path."""


-- 
Matthieu