A callback function in sftp_client.get() fails when downloading a 0 byte file. #90

enB-zz opened this Issue Oct 13, 2012 · 2 comments


None yet

2 participants

enB-zz commented Oct 13, 2012

It's caused by the following being above the first call of callback. The function never finds the size of the files being downloaded.

if len(data) == 0:

Changing the loop allows the callback to know that the size of the file being downloaded is 0 bytes.

while True:
    data = fr.read(32768)
    if callback is not None: # Now being called before the loop is exited.
        callback(size, file_size)
    if len(data) == 0:
    size += len(data)

Confirmed the problem, via a temporary patch to Fabric (should probably merge some of the more robust patches along these lines that have been in that tracker for a while! 😔).

Not sure your suggested patch is the best approach -- it changes how the callback reports things in the previously-working case, insofar as it now reports the initial "read 0/xxx bytes" always, vs before where the first hit to the callback would be e.g. "read 128/xxx bytes" or however many it managed to read on the first iteration.

Is that likely to make anybody's code break? Hopefully not, but still less than ideal.

I think a better solution is to move the size incrementing along with the callback test; that preserves the old behavior in the case where the file is not zero bytes, but still gives us the "calls callback once with 0/0 args" behavior you need added. Testing this now.


Actually, it's even easier than that -- simply move the break test to the end of the loop. Writing out an empty string to the local file shouldn't hurt anybody, especially since this code already appears to create an empty local file in the current implementation.

@bitprophet bitprophet added a commit that closed this issue Oct 15, 2012
@bitprophet bitprophet Move SFTPClient.get() termination condition to loop end.
Ensures callback always executes even for zero-len files.

Fixes #90
@bitprophet bitprophet added a commit that referenced this issue Nov 30, 2012
@bitprophet bitprophet Apply put() version of #90 2cbe383
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment