[rfc] [patch] pycurl transport

[John Arbash Meinel]

Martin Pool wrote:
> On 12 Jan 2006, Robert Collins <robertc at robertcollins.net> wrote:
> 
> Thanks for reviewing it.
...

>>>+    def get(self, relpath, decode=False):
>>>+        if decode:
>>>+            raise NotImplementedError
>>>+        return self._get_url(self.abspath(relpath))
>>
>>Whats decode here? Its not in the main transport api .... and having it
>>raise makes it look rather - uhm - unneeded ?
> 
> 
> It was present in HttpTransport.  Perhaps it's obsolete/unused; I'll
> check.

Decode used to be in the transport api. But hasn't been for a long time.
The parameter isn't even used in HttpTransport, and should be removed.


> 
> It was getting a Unicode object when running the tests, which is why I 
> added this.  Apparently it was only ever a Unicode containing ascii
> characters though.  Perhaps I should change this to just raise an
> assertion and fix the callers.
> 
> 
>>.. These should go into mainline regardless - +1 on them ;)
> 
> 
> OK, good.  The messages are much clearer, showing the method name and
> instance class.
> 

That is nice. I didn't know that you could do it.

> 
>>this looks good +1 with the above tweaks ...
>>
>>some small extra thoughts:
>>perhaps it would be cleaner to do:
>>mkdir bzrlib/transport/http
>>bzr add bzrlib/transport/http
>>bzr mv bzrlib/transport/http.py bzrlib/transport/http/__init__.py
>>
>>Then, have _pycurl.py and _urllib.py containing the respective pycurl
>>and urllib implementations, with the common code in __init__.py
> 
> 
> Yes.  
> 
> I had avoided those names because of concern they would clash with the
> possible C impl of pycurl, but I guess that will be imported from a
> different module and so not really clash.
> 

As long as you don't do 'import _pycurl' but do 'import
bzrlib.transport.http._pycurl'.
But honestly, if they are in the bzrlib.transport.http interface, why do
we need the underscore?

> 
>>As for which implementation is default, if we change the protocol
>>handler to allow a *list* of handlers for a prefix, then we can just
>>probe until one allows itself to be constructed, and if none do, then
>>complain/provide the last error.
> 
> 
> Yep.  I'll send a patch for that part for review.
> 

As long as there is an obvious priority. Otherwise you might always try
to use urllib, even though you have pycurl installed.

It also is pretty trivial to do the same thing that we do in the test suite.

if has_pycurl:
  register_transport...

The only reason we do the crazy stuff with paramiko, is that we didn't
want to rewrite the code in a big if/then, since we had classes which
had to be children of paramiko classes.

John
=:->
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 256 bytes
Desc: OpenPGP digital signature
Url : https://lists.ubuntu.com/archives/bazaar/attachments/20060111/d651e0e1/attachment.pgp