[PATCH] knit pushing with ftp transport
Alexandre Saint
stalst at gmail.com
Mon May 1 18:57:10 BST 2006
On Mon, May 01, 2006 at 09:56:20AM -0500, John Arbash Meinel wrote:
[...]
> Looks pretty good. One issue is that you have embedded tab characters,
> rather than only whitespace. PEP8 says space characters only.
>
Pep8'ified the patch.
[...]
> We try to have a single line summary at the beginning. Something like:
>
> """Try repeatedly to append the given text to the file at relpath.
>
> More information here.
> """
>
Ok. Better documented now.
> > + try:
> > + abspath = self._abspath(relpath)
> > + mutter("FTP appe to %s" % abspath)
>
> mutter() and all of the logging/debug functions are designed to take a
> list of arguments, and format the string. Also, since this is in the
> recursive portion, I would do:
>
> mutter("FTP appe (%d) to %s", retries, abspath)
>
[...]
> I do believe warning() should be done the same:
> warning("FTP temporary error: %s. Retrying.", str(e))
>
> This is a small performance issue. Technically mutter() may not do
> anything if the debug level isn't low enough. And in that case it
> doesn't need to take the time to format the string, also it helps to
> catch UnicodeErrors, since they happen inside mutter() instead of inside
> your own code.
>
Adapted all mutter() and warning() calls throughout ftp.py, in this patch.
> +1 with a couple of fixes. Someone else needs to review it before it can
> be merged, and somehow I managed to upset the pqm so it isn't able to
> merge from me right now. So someone else needs to submit it anyway.
>
> John
> =:->
>
Thanks, John :).
So seeking +1 from other people, now. :)
Regards,
--
alex
-------------- next part --------------
=== modified file 'a/bzrlib/transport/ftp.py'
--- a/bzrlib/transport/ftp.py
+++ b/bzrlib/transport/ftp.py
@@ -47,7 +47,7 @@
"""Find an ftplib.FTP instance attached to this triplet."""
key = "%s|%s|%s|%s" % (hostname, username, password, is_active)
if key not in _FTP_cache:
- mutter("Constructing FTP instance against %r" % key)
+ mutter("Constructing FTP instance against %r", key)
_FTP_cache[key] = ftplib.FTP(hostname, username, password)
_FTP_cache[key].set_pasv(not is_active)
return _FTP_cache[key]
@@ -173,10 +173,10 @@
try:
f = self._get_FTP()
s = f.size(self._abspath(relpath))
- mutter("FTP has: %s" % self._abspath(relpath))
+ mutter("FTP has: %s", self._abspath(relpath))
return True
except ftplib.error_perm:
- mutter("FTP has not: %s" % self._abspath(relpath))
+ mutter("FTP has not: %s", self._abspath(relpath))
return False
def get(self, relpath, decode=False, retries=0):
@@ -191,7 +191,7 @@
"""
# TODO: decode should be deprecated
try:
- mutter("FTP get: %s" % self._abspath(relpath))
+ mutter("FTP get: %s", self._abspath(relpath))
f = self._get_FTP()
ret = StringIO()
f.retrbinary('RETR '+self._abspath(relpath), ret.write, 8192)
@@ -205,7 +205,7 @@
% self.abspath(relpath),
orig_error=e)
else:
- warning("FTP temporary error: %s. Retrying." % str(e))
+ warning("FTP temporary error: %s. Retrying.", str(e))
self._FTP_instance = None
return self.get(relpath, decode, retries+1)
except EOFError, e:
@@ -234,7 +234,7 @@
if not hasattr(fp, 'read'):
fp = StringIO(fp)
try:
- mutter("FTP put: %s" % self._abspath(relpath))
+ mutter("FTP put: %s", self._abspath(relpath))
f = self._get_FTP()
try:
f.storbinary('STOR '+tmp_abspath, fp)
@@ -244,8 +244,8 @@
try:
f.delete(tmp_abspath)
except:
- warning("Failed to delete temporary file on the server.\nFile: %s"
- % tmp_abspath)
+ warning("Failed to delete temporary file on the" \
+ "server.\nFile: %s", tmp_abspath)
raise e
raise
except ftplib.error_perm, e:
@@ -259,7 +259,7 @@
raise TransportError("FTP temporary error during PUT %s. Aborting."
% self.abspath(relpath), orig_error=e)
else:
- warning("FTP temporary error: %s. Retrying." % str(e))
+ warning("FTP temporary error: %s. Retrying.", str(e))
self._FTP_instance = None
self.put(relpath, fp, mode, retries+1)
except EOFError:
@@ -276,7 +276,7 @@
def mkdir(self, relpath, mode=None):
"""Create a directory at the given path."""
try:
- mutter("FTP mkd: %s" % self._abspath(relpath))
+ mutter("FTP mkd: %s", self._abspath(relpath))
f = self._get_FTP()
try:
f.mkd(self._abspath(relpath))
@@ -292,7 +292,7 @@
def rmdir(self, rel_path):
"""Delete the directory at rel_path"""
try:
- mutter("FTP rmd: %s" % self._abspath(rel_path))
+ mutter("FTP rmd: %s", self._abspath(rel_path))
f = self._get_FTP()
f.rmd(self._abspath(rel_path))
@@ -303,11 +303,67 @@
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
+
+ mutter("FTP appe to %s", self._abspath(relpath))
+ self._try_append(relpath, f.read(), mode)
+
+ return result
+
+ def _try_append(self, relpath, text, mode=None, retries=0):
+ """Try repeatedly to append the given text to the file at relpath.
+
+ This is a recursive function. On errors, it will be called until the
+ number of retries is exceeded.
+ """
+ try:
+ abspath = self._abspath(relpath)
+ mutter("FTP appe (try %d) to %s", retries, 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 supports the 'SITE CHMOD'
+ extension.
+ """
+ try:
+ mutter("FTP site chmod: setting permissions to %s on %s",
+ str(mode), self._abspath(relpath))
+ 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
+ warning("FTP Could not set permissions to %s on %s. %s",
+ str(mode), self._abspath(relpath), str(e))
def copy(self, rel_from, rel_to):
"""Copy the item at rel_from to the location at rel_to"""
@@ -316,8 +372,8 @@
def move(self, rel_from, rel_to):
"""Move the item at rel_from to the location at rel_to"""
try:
- mutter("FTP mv: %s => %s" % (self._abspath(rel_from),
- self._abspath(rel_to)))
+ mutter("FTP mv: %s => %s", self._abspath(rel_from),
+ self._abspath(rel_to))
f = self._get_FTP()
f.rename(self._abspath(rel_from), self._abspath(rel_to))
except ftplib.error_perm, e:
@@ -328,7 +384,7 @@
def delete(self, relpath):
"""Delete the item at relpath"""
try:
- mutter("FTP rm: %s" % self._abspath(relpath))
+ mutter("FTP rm: %s", self._abspath(relpath))
f = self._get_FTP()
f.delete(self._abspath(relpath))
except ftplib.error_perm, e:
@@ -344,14 +400,15 @@
def list_dir(self, relpath):
"""See Transport.list_dir."""
try:
- mutter("FTP nlst: %s" % self._abspath(relpath))
+ 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)
@@ -374,7 +431,7 @@
"""Return the stat information for a file.
"""
try:
- mutter("FTP stat: %s" % self._abspath(relpath))
+ mutter("FTP stat: %s", self._abspath(relpath))
f = self._get_FTP()
return FtpStatResult(f, self._abspath(relpath))
except ftplib.error_perm, 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/df3089a3/attachment.pgp
More information about the bazaar
mailing list