[PATCH] knit pushing with ftp transport

[Alexandre Saint]

On Mon, May 01, 2006 at 07:51:48AM -0500, John Arbash Meinel wrote:
> Alexandre Saint wrote:
> > On Sun, Apr 30, 2006 at 03:34:02PM -0500, John Arbash Meinel wrote:
> >> Alexandre Saint wrote:
> >>> Hello,
> >>>
> >>> This patch adds support for knit format to the ftp transport.
> >>>
> >>> Added an append() method using FTP 'APPE' command.
> >>> Modified copy() to deal well with relative paths (and not only with
> >>> absolute ones).
> >>>
[...] 
> The interface for append() has to be a file-like object (as defined by
> Transport).
> 
> I was suggesting that append() should not be recursive, but should call
> something like _append_helper() which would be recursive, and would
> expect a string.
> 

I misunderstood you. But I agree know.

> We decided on a policy of "no magic interfaces", so we don't want to use
> isinstance() switches if we can help it.
> 
> Oh, and the correct way to determine it is "isinstance(x, str)". If you
> want to compare for both regular strings and unicode strings, you need
> to use "isinstance(x, basestring)", but in this case you don't want to
> accept unicode strings.
> 
> I would do something more like:
> 
> def append(self, relpath, f, mode=None):
> 	if self.has(relpath):
> 		result = ftp.size(abspath)
> 	else:
> 		result = 0
> 	self._append_helper(self._abspath(relpath), f.read(),
> 		mode=mode, retries=0)
> 
> 	return result
> 
> def _append_helper(self, abspath, f, mode=None, retries=0):
> 	"""This is a recursive function that will ..."
> 
> 
> I'm not sure if you need to check the file size each time, or just once.
> But things that don't need to be repeated (like _abspath, and f.read())
> would go into 'append', and the repeating stuff would go into
> _append_helper().
> Or maybe it should be called _try_append()
> 
> John
> =:->
> 

In this patch, added _try_append(), which is recursive.

-- 
alex
-------------- next part --------------
=== modified file 'a/bzrlib/transport/ftp.py'
--- a/bzrlib/transport/ftp.py	
+++ b/bzrlib/transport/ftp.py	
@@ -303,11 +303,64 @@
                 raise TransportError(msg="Cannot remove directory at %s" % \
                         self._abspath(rel_path), extra=str(e))
 
-    def append(self, relpath, f):
+    def append(self, relpath, f, mode=None):
         """Append the text in the file-like object into the final
         location.
         """
-        raise TransportNotPossible('ftp does not support append()')
+        if self.has(relpath):
+	    ftp = self._get_FTP()
+            result = ftp.size(self._abspath(relpath))
+        else:
+            result = 0
+
+	self._try_append(relpath, f.read(), mode)
+
+	return result
+
+    def _try_append(self, relpath, text, mode=None, retries=0):
+        """This is a recursive function that will try to append the text
+	given as argument to the final location until the number of retries
+	exceeds the limit.
+	"""
+        try:
+            abspath = self._abspath(relpath)
+            mutter("FTP appe to %s" % abspath)
+            ftp = self._get_FTP()
+            ftp.voidcmd("TYPE I")
+            cmd = "APPE %s" % abspath
+            conn = ftp.transfercmd(cmd)
+            conn.sendall(text)
+            conn.close()
+            if mode is not None:
+                self._setmode(relpath, mode)
+            ftp.getresp()
+        except ftplib.error_perm, e:
+            FtpTransportError("Error appending data to %s" % abspath,
+                    orig_error=e)
+        except ftplib.error_temp, e:
+            if retries > _number_of_retries:
+                raise TransportError("FTP temporary error during APPEND %s." \
+                        "Aborting." % abspath, orig_error=e)
+            else:
+                warning("FTP temporary error: %s. Retrying." % str(e))
+                self._FTP_instance = None
+                self._try_append(relpath, text, mode, retries+1)
+
+
+    def _setmode(self, relpath, mode):
+        """Set permissions on a path.
+
+        Only set permissions if the FTP server support the 'SITE CHMOD'
+        extension.
+        """
+        try:
+            ftp = self._get_FTP()
+            cmd = "SITE CHMOD %s %s" % (self._abspath(relpath), str(mode))
+            ftp.sendcmd(cmd)
+        except ftplib.error_perm, e:
+            # Command probably not available on this server
+            pass
+
 
     def copy(self, rel_from, rel_to):
         """Copy the item at rel_from to the location at rel_to"""
@@ -347,11 +400,12 @@
             mutter("FTP nlst: %s" % self._abspath(relpath))
             f = self._get_FTP()
             basepath = self._abspath(relpath)
-            # FTP.nlst returns paths prefixed by relpath, strip 'em
-            the_list = f.nlst(basepath)
-            stripped = [path[len(basepath)+1:] for path in the_list]
+            paths = f.nlst(basepath)
+            # If FTP.nlst returns paths prefixed by relpath, strip 'em
+            if paths[0].startswith(basepath):
+                paths = [path[len(basepath)+1:] for path in paths]
             # Remove . and .. if present, and return
-            return [path for path in stripped if path not in (".", "..")]
+            return [path for path in paths if path not in (".", "..")]
         except ftplib.error_perm, e:
             raise TransportError(orig_error=e)
 

-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 191 bytes
Desc: not available
Url : https://lists.ubuntu.com/archives/bazaar/attachments/20060501/4e297d2e/attachment.pgp