Rev 4726: Fix test failure and clean up ftp APPE. in file:///home/vila/src/bzr/bugs/443041-ftp-append-bytes/
Vincent Ladeuil
v.ladeuil+lp at free.fr
Mon Oct 5 15:29:46 BST 2009
At file:///home/vila/src/bzr/bugs/443041-ftp-append-bytes/
------------------------------------------------------------
revno: 4726
revision-id: v.ladeuil+lp at free.fr-20091005142946-26vzznan0kgxlq11
parent: pqm at pqm.ubuntu.com-20091002180105-oeomf71t7l9gpv6g
committer: Vincent Ladeuil <v.ladeuil+lp at free.fr>
branch nick: 443041-ftp-append-bytes
timestamp: Mon 2009-10-05 16:29:46 +0200
message:
Fix test failure and clean up ftp APPE.
* bzrlib/transport/ftp/__init__.py:
(FtpTransport._try_append): Bad usage of the library and mixed
dialogs left the connection is ASCII mode at the wrong time.
(FtpTransport._try_append, FtpTransport._fallback_append): Use a
file-like object instead of a str parametre to avoid useless
buffering.
(FtpTransport.append_file): Don't read the whole text to append,
leave the called functions handled that instead.
* bzrlib/tests/ftp_server/pyftpdlib_based.py:
(FTPTestServer._run_server): Don't let spurious EBADF pollute the
test outputs, they are rare but it's useless to report them.
-------------- next part --------------
=== modified file 'NEWS'
--- a/NEWS 2009-10-02 16:48:36 +0000
+++ b/NEWS 2009-10-05 14:29:46 +0000
@@ -101,6 +101,9 @@
with some combinations of remote and local formats. This was causing
"unknown object type identifier 60" errors. (Andrew Bennetts, #427736)
+* ftp APPE command was not cleanly handled in all cases and buffered too
+ much too. (Vincent Ladeuil, #443041)
+
* Network streams now decode adjacent records of the same type into a
single stream, reducing layering churn. (Robert Collins)
=== modified file 'bzrlib/tests/ftp_server/pyftpdlib_based.py'
--- a/bzrlib/tests/ftp_server/pyftpdlib_based.py 2009-03-23 14:59:43 +0000
+++ b/bzrlib/tests/ftp_server/pyftpdlib_based.py 2009-10-05 14:29:46 +0000
@@ -202,7 +202,11 @@
self._ftpd_running = True
self._ftpd_starting.release()
while self._ftpd_running:
- self._ftp_server.serve_forever(timeout=0.1, count=1)
+ try:
+ self._ftp_server.serve_forever(timeout=0.1, count=1)
+ except select.error, e:
+ if e.args[0] != errno.EBADF:
+ raise
self._ftp_server.close_all(ignore_all=True)
def add_user(self, user, password):
=== modified file 'bzrlib/transport/ftp/__init__.py'
--- a/bzrlib/transport/ftp/__init__.py 2009-07-22 06:51:13 +0000
+++ b/bzrlib/transport/ftp/__init__.py 2009-10-05 14:29:46 +0000
@@ -25,17 +25,13 @@
"""
from cStringIO import StringIO
-import errno
import ftplib
import getpass
import os
-import os.path
-import urlparse
import random
import socket
import stat
import time
-from warnings import warn
from bzrlib import (
config,
@@ -51,8 +47,6 @@
register_urlparse_netloc_protocol,
Server,
)
-from bzrlib.transport.local import LocalURLServer
-import bzrlib.ui
register_urlparse_netloc_protocol('aftp')
@@ -99,8 +93,9 @@
self.is_active = True
else:
self.is_active = False
-
- # Most modern FTP servers support the APPE command. If ours doesn't, we (re)set this flag accordingly later.
+
+ # Most modern FTP servers support the APPE command. If ours doesn't, we
+ # (re)set this flag accordingly later.
self._has_append = True
def _get_FTP(self):
@@ -390,8 +385,6 @@
"""Append the text in the file-like object into the final
location.
"""
- text = f.read()
-
abspath = self._remote_path(relpath)
if self.has(relpath):
ftp = self._get_FTP()
@@ -401,13 +394,13 @@
if self._has_append:
mutter("FTP appe to %s", abspath)
- self._try_append(relpath, text, mode)
+ self._try_append(relpath, f, mode)
else:
- self._fallback_append(relpath, text, mode)
+ self._fallback_append(relpath, f, mode)
return result
- def _try_append(self, relpath, text, mode=None, retries=0):
+ def _try_append(self, relpath, fp, 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
@@ -417,37 +410,37 @@
abspath = self._remote_path(relpath)
mutter("FTP appe (try %d) to %s", retries, abspath)
ftp = self._get_FTP()
- cmd = "APPE %s" % abspath
- conn = ftp.transfercmd(cmd)
- conn.sendall(text)
- conn.close()
+ starting_at = fp.tell()
+ ftp.storbinary("APPE %s" % abspath, fp)
self._setmode(relpath, mode)
- ftp.getresp()
except ftplib.error_perm, e:
# Check whether the command is not supported (reply code 502)
if str(e).startswith('502 '):
- warning("FTP server does not support file appending natively. " \
+ warning(
+ "FTP server does not support file appending natively. "
"Performance may be severely degraded! (%s)", e)
self._has_append = False
- self._fallback_append(relpath, text, mode)
+ fp.seek(starting_at)
+ self._fallback_append(relpath, fp, mode)
else:
self._translate_perm_error(e, abspath, extra='error appending',
unknown_exc=errors.NoSuchFile)
-
except ftplib.error_temp, e:
if retries > _number_of_retries:
- raise errors.TransportError("FTP temporary error during APPEND %s." \
- "Aborting." % abspath, orig_error=e)
+ raise errors.TransportError(
+ "FTP temporary error during APPEND %s. Aborting."
+ % abspath, orig_error=e)
else:
warning("FTP temporary error: %s. Retrying.", str(e))
self._reconnect()
- self._try_append(relpath, text, mode, retries+1)
+ fp.seek(starting_at)
+ self._try_append(relpath, fp, mode, retries+1)
- def _fallback_append(self, relpath, text, mode = None):
+ def _fallback_append(self, relpath, fp, mode = None):
remote = self.get(relpath)
- remote.seek(0, 2)
- remote.write(text)
- remote.seek(0, 0)
+ remote.seek(0, os.SEEK_END)
+ osutils.pumpfile(fp, remote)
+ remote.seek(0)
return self.put_file(relpath, remote, mode)
def _setmode(self, relpath, mode):
More information about the bazaar-commits
mailing list