[PATCH] knit pushing with ftp transport

[John Arbash Meinel]

Alexandre Saint wrote:
> On Sun, Apr 30, 2006 at 03:34:02PM -0500, John Arbash Meinel wrote:
>> Alexandre Saint wrote:
>>> Hello,
>>>
>>> This patch adds support for knit format to the ftp transport.
>>>
>>> Added an append() method using FTP 'APPE' command.
>>> Modified copy() to deal well with relative paths (and not only with
>>> absolute ones).
>>>
>>> Regards.
>>>
>>>
>> I just realized something while looking over this code. Did we ever
>> specify that the File-like object needs to be seekable? And that we must
>> be able to backtrack? (One example is you cannot seek stdin).
>>
>> Since the line where you copy the bytes is just doing:
>> 	conn.sendall(f.read())
>> I'm wondering if it wouldn't be better to change things, and have the
>> helper function (which gets retries), take a string buffer, rather than
>> seeking on the file object.
>>
> Changed made according to your suggestion.
> 
> Though, is 'isinstance(text, str)' the good way to check if we are given
> a str object or not?
> 
>> Other than that, +1 from me.
>>
>> John
>> =:->
>>
>>

The interface for append() has to be a file-like object (as defined by
Transport).

I was suggesting that append() should not be recursive, but should call
something like _append_helper() which would be recursive, and would
expect a string.

We decided on a policy of "no magic interfaces", so we don't want to use
isinstance() switches if we can help it.

Oh, and the correct way to determine it is "isinstance(x, str)". If you
want to compare for both regular strings and unicode strings, you need
to use "isinstance(x, basestring)", but in this case you don't want to
accept unicode strings.

I would do something more like:

def append(self, relpath, f, mode=None):
	if self.has(relpath):
		result = ftp.size(abspath)
	else:
		result = 0
	self._append_helper(self._abspath(relpath), f.read(),
		mode=mode, retries=0)

	return result

def _append_helper(self, abspath, f, mode=None, retries=0):
	"""This is a recursive function that will ..."


I'm not sure if you need to check the file size each time, or just once.
But things that don't need to be repeated (like _abspath, and f.read())
would go into 'append', and the repeating stuff would go into
_append_helper().
Or maybe it should be called _try_append()

John
=:->

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 254 bytes
Desc: OpenPGP digital signature
Url : https://lists.ubuntu.com/archives/bazaar/attachments/20060501/0a7d3e56/attachment.pgp