[RFC] bzr+ssh without paramiko

John Arbash Meinel john at arbash-meinel.com
Wed Nov 8 18:12:17 GMT 2006


Martin Pool wrote:
> On 31 Oct 2006, John Arbash Meinel <john at arbash-meinel.com> wrote:
>> Theoretically, we don't need paramiko if we have an ssh process that we
>> can spawn.
>>
>> The attached patch changes bzrlib/transport/ssh.py so that if paramiko
>> isn't present, it still tries to run.
>>
>> In simple testing bzr+ssh:// does work.
> 
> +1
> 

...

> 
>> Also, things may not fail as nicely as they could when paramiko isn't
>> installed. sftp.py still gives the same exception, though. So it
>> shouldn't effect those things.
> 
> To confirm: sftp urls will still fail reasonably if you have ssh but not
> paramiko?
> 

Yes, sftp:// urls fail correctly. You can actually test this with:

% echo "raise ImportError('no paramiko')" > paramiko.py
% ./bzr log sftp://host/...
bzr: ERROR: Unsupported protocol for url
"sftp://juju/srv/bzr/public/branches/bzr/jam-integration": Unable to
import paramiko (required for sftp support): no paramiko

We can actually easily run the test suite this way if we wanted. We
could do it for both pycurl and for paramiko. Something like:
=== modified file 'Makefile'
--- Makefile    2006-11-02 07:01:58 +0000
+++ Makefile    2006-11-08 17:52:49 +0000
@@ -5,6 +5,9 @@
        @echo "Running all tests with no locale."
        LC_CTYPE= LANG=C LC_ALL= ./bzr selftest -v $(tests)
        python -O -Werror ./bzr selftest -v $(tests)
+       echo "raise ImportError('no paramiko')" > paramiko.py
+       echo "raise ImportError('no pycurl')" > pycurl.py
+       python -Werror ./bzr selftest -v $(tests)

 check-msgeditor:
        ./bzr --no-plugins selftest -v msgeditor

I don't know that we really want to do it, but that should run our test
suite with paramiko and pycurl disabled.

The reason it is a little hackish, is because there are code paths
inside that file that just access paramiko directly, or even SFTPClient,
which is no longer being imported.

Now, these shouldn't be used if paramiko is not available, because we
don't install the ParamikoVendor if we can't import paramiko, etc. And
if someone imports 'bzrlib.transport.sftp' it still gives the same error.

I just wrote a follow up, which changes it to raise ParamikoNotPresent
before any code path that uses 'paramiko'. Which would raise a nicer
error than AttributeError. However, it is a little ugly, and means that
we are redundantly checking in a lot of places. Which makes me feel like
this stuff should really be layered differently.

So I'll merge the first version, and we can decide if we want the
extended version.

John
=:->
-------------- next part --------------
A non-text attachment was scrubbed...
Name: bzr_ssh_no_paramiko.patch
Type: text/x-patch
Size: 5856 bytes
Desc: not available
Url : https://lists.ubuntu.com/archives/bazaar/attachments/20061108/37508566/attachment.bin 
-------------- 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/20061108/37508566/attachment.pgp 


More information about the bazaar mailing list