duplicity-talk
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Duplicity-talk] more polished backends.py/ftpBackend (was: 0.4.3.RC


From: dAniel hAhler
Subject: Re: [Duplicity-talk] more polished backends.py/ftpBackend (was: 0.4.3.RC3 Ready for Test)
Date: Fri, 1 Jun 2007 22:04:30 +0200

Kenneth Loafman wrote:
> Please find attached a new backends.py file and a patch against CVS HEAD:
> - use self.ftp.quit() in self.close()
> - use close() in connect() and only "pass" for ftplib.all_errors exceptions
> - Log FTP command in the retry-loop - so the used comment gets logged
> also when retrying
> - Optimized "nlst" check
> - Removed handling of 221/421 errors - this should get handled now in
> general
> - Whitespace fixes for log.FatalError; also print stack with the last one
> - Do not sleep on the first retry: this avoids a 10 seconds delay on
> e.g. EOFError
> - Just call connect() if we expect to be logged in; moved this out of
> the try block
>
> The main fix probably is: call connect() on errors, if is_connected -
> this means: we expect to be connected.
> As connect() now disconnects/quits first, this should fix the "too
> many logins" issue found recently - and which had been fixed by you in
> some other way.
> Also the "only sleep 0 seconds on first exception" change is a good
> one: we should not wait before re-trying the first time.
>
> There are also some TEST and TODO comments. Please review either the
> patch and apply it as you think it fits. I would just drop the TEST
> comment, which is mainly a hint for testing the backend also with a
> good connection ;)

OK, this will work, but I have philosophical problems with handling all
errors with a sledge hammer, however, as funky as FTP is implemented in
general, this may work out to be the best solution.

Do you refer to the special 221/421 error handling? I think it may
make sense to leave them in and/or have special handling for some
error codes, but just removing them seemed to be the safest method for
this patch.

Was there a reason you removed the change that would turn the passive
mode off in NLST (425 error, line 460)?

Oh, sorry. I've removed it on my working copy, because of the
set_pasv() problems I've reported already. You said to make it more
robust for the release anyway - so that's your chance.. ;)




reply via email to

[Prev in Thread] Current Thread [Next in Thread]