Code review comment for lp:~ndurner/bzr/bzr-ftp

Revision history for this message
Martin Pool (mbp) wrote :

> Fixes bug #294709.
> Due to time constraints, I haven't looked into writing a testcase for it yet.

8 +
9 + self._has_append = True

OK it's fairly obvious but a comment here would be good, especially because it's "might have append", not necessarily "has append".

39 except ftplib.error_perm, e:
40 - self._translate_perm_error(e, abspath, extra='error appending',
41 - unknown_exc=errors.NoSuchFile)
42 + warning("FTP server does not support file appending natively. " \
43 + "Performance may be severely degraded!")
44 + self._has_append = False
45 + self._fallback_append(relpath, text, mode)
46 +

This seems to be treating *any* permission error on append as being failure to support append. What about if we just can't append to that one particular file because of say a permissions problem? I guess you need to either look for an error code or perhaps match the string.

Also to help debug this it might be good to include e in the message.

review: Needs Fixing

« Back to merge proposal