[merge] Transport.{append,get,put}_bytes

[John Arbash Meinel]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Robert Collins wrote:
> On Thu, 2006-08-24 at 14:32 -0500, John Arbash Meinel wrote:
>> Attached is a patch which implements and tests 3 new functions on
>> Transport. append_bytes, get_bytes, and put_bytes.
>>
>> There is a base-level implementation which just takes the bytes, and
>> wraps them in a cStringIO.StringIO() object, and calls the normal
>> function. This means that all Transports already have them implemented,
>> and can chose if they want to special case it.
>>
>> It also fixes a couple of calls where we were passing a string to
>> 'put()' rather than passing a file-like object.
>>
>> The biggest benefit for us is that we have a lot of calls that already
>> have a string in memory, and it is a lot of busy work for all of those
>> locations to wrap their strings in StringIO().
>>
>> Also, we would like to deprecate put() taking strings, but that needs to
>> wait until the next release. This release just gives us the alternative,
>> and we can do the deprecation dance later.
>>
>> I also cleaned up the put and append tests. We had a lot of bogus
>> redundancy because I wrote the tests a long time ago, when I didn't
>> write simple tests. :) Also, at one point append() allowed both strings
>> and file-like objects, but that api was de-magicked a while ago. 'put()'
>> was intended to be de-magicked, but we obviously didn't do it thoroughly.
>>
>> On IRC, Robert thought this could be +1 for 0.10, since it is a
>> relatively small api extension, with no loss of functionality.
>>
>> Also, we probably want to deprecated the get_multi, put_multi, and
>> append_multi (copy_multi?) apis. They never caught on, so right now the
>> only thing using them is the test suite. (delete_multi and mkdir_multi
>> *are* used, but probably could be replaced)
> 
> On balance, not for 0.10.
> 
> For 0.11, I have some comments..
> 
> It might be nicer to do the following:
> 
> deprecate put() completely.
> Add put_bytes AND put_file.

Possibly. I think plain put() is still reasonable, though.

> 
> Then the hunks in your patch that look like:
> 
> === modified file bzrlib/branch.py
> --- bzrlib/branch.py
> +++ bzrlib/branch.py
> @@ -1248,7 +1248,7 @@
>                          "use bzrlib.urlutils.escape")
>                      
>              url = urlutils.relative_url(self.base, url)
> -            self.control_files.put('parent', url + '\n')
> +            self.control_files.put('parent', StringIO(url + '\n'))
> 
> Would be:
> 
> === modified file bzrlib/branch.py
> --- bzrlib/branch.py
> +++ bzrlib/branch.py
> @@ -1248,7 +1248,7 @@
>                          "use bzrlib.urlutils.escape")
>                      
>              url = urlutils.relative_url(self.base, url)
> -            self.control_files.put('parent', url + '\n')
> +            self.control_files.put_bytes('parent', url + '\n')
> 

^- Actually, 'control_files' is a LockableFiles, not a Transport. So it
would have to be upgraded as well. (we have put() and put_utf8(), urls
are technically ascii, so they *could* work as put_utf8, but it isn't
quite right)

> 
> And there would be no strange-errors thrown from put after we finish the
> conversion - it would just be another deprecated function, which would
> call either put_file or put_bytes.
> 
> -Rob
>  

Do you want to change append as well then? I don't know that anyone is
passing a string, but since Transport._pump() supports both, I'm pretty
sure it would have worked before we switched to _pump_to_fd.

John
=:->
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.0 (Darwin)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iD8DBQFE8j8YJdeBCYSNAAMRAt1ZAKCfbSrD94tv6aPkPcZBdSRWPbtHCgCgpq/X
I1ASH7Tkw7IX12WOoa1unj8=
=d5q/
-----END PGP SIGNATURE-----