Bazaar

Merge lp:~spiv/bzr/ssh-connect-socket-error into lp:bzr

  1. ssh-connect-socket-error
  2. Merge into bzr.dev
Proposed by Andrew Bennetts
Status: Merged
Approved by: Vincent Ladeuil
Approved revision: no longer in the source branch.
Merged at revision: 5438
Proposed branch: lp:~spiv/bzr/ssh-connect-socket-error
Merge into: lp:bzr
Diff against target: 67 lines (+7/-19)
2 files modified
NEWS (+3/-0)
bzrlib/transport/ssh.py (+4/-19)
To merge this branch: bzr merge lp:~spiv/bzr/ssh-connect-socket-error
Reviewer Review Type Date Requested Status
Vincent Ladeuil Approve
Review via email: mp+35784@code.launchpad.net

Commit message

Simplify connect_ssh/sftp error handling

Description of the change

test_bad_ssh_connection is still failing intermittently: http://babune.ladeuil.net:24842/job/selftest-hardy/lastFailedBuild/testReport/junit/bzrlib.tests.test_sftp_transport/SSHVendorBadConnection/test_bad_connection_ssh/

Looking at the code, I don't know what the separate handling of EPIPE is trying to achieve, so here's a patch to just keep the error handling during connect_ssh/sftp simple and consistent.

To post a comment you must log in.
Revision history for this message
Vincent Ladeuil (vila) wrote :

Let's try that and monitor the failures on hardy.

AIUI this shouldn't occur outside of tests so is pretty safe to try.

review: Approve
Revision history for this message
Andrew Bennetts (spiv) wrote :

Vincent Ladeuil wrote:
> Review: Approve
> Let's try that and monitor the failures on hardy.
>
> AIUI this shouldn't occur outside of tests so is pretty safe to try.

I'm not as confident about that, but I think I've forgotten too much of
the original context to be confident of anything here :/

Revision history for this message
Vincent Ladeuil (vila) wrote :

At least this didn't fail any tests on: http://babune.ladeuil.net:24842/view/debug/job/selftest-subset-all/lastCompletedBuild/aggregatedTestReport/ for -s bzrlib.tests.test_sftp_transport

Revision history for this message
Andrew Bennetts (spiv) wrote :

sent to pqm by email

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'NEWS'
2--- NEWS 2010-09-15 10:10:22 +0000
3+++ NEWS 2010-09-17 07:43:44 +0000
4@@ -183,6 +183,9 @@
5 be available from ``bzrlib.tests.*``. They used to be, but were
6 accidentally removed. (John Arbash Meinel, #627438)
7
8+* Treat all IO, OS, and socket errors consistently when establishing
9+ SSH/SFTP connections via a subprocess. (Andrew Bennetts)
10+
11 * ``Transport.stat`` on a symlink, including a transport pointing directly
12 to a symlink, now returns information about the symlink.
13 (Martin Pool)
14
15=== modified file 'bzrlib/transport/ssh.py'
16--- bzrlib/transport/ssh.py 2010-09-15 10:10:22 +0000
17+++ bzrlib/transport/ssh.py 2010-09-17 07:43:44 +0000
18@@ -336,15 +336,14 @@
19 self._raise_connection_error(host, port=port, orig_error=e,
20 msg='Unable to invoke remote bzr')
21
22+_ssh_connection_errors = (EOFError, OSError, IOError, socket.error)
23 if paramiko is not None:
24 vendor = ParamikoVendor()
25 register_ssh_vendor('paramiko', vendor)
26 register_ssh_vendor('none', vendor)
27 register_default_ssh_vendor(vendor)
28- _sftp_connection_errors = (EOFError, paramiko.SSHException)
29+ _ssh_connection_errors += (paramiko.SSHException,)
30 del vendor
31-else:
32- _sftp_connection_errors = (EOFError,)
33
34
35 class SubprocessVendor(SSHVendor):
36@@ -375,14 +374,7 @@
37 subsystem='sftp')
38 sock = self._connect(argv)
39 return SFTPClient(SocketAsChannelAdapter(sock))
40- except _sftp_connection_errors, e:
41- self._raise_connection_error(host, port=port, orig_error=e)
42- except (OSError, IOError, socket.error), e:
43- # If the machine is fast enough, ssh can actually exit
44- # before we try and send it the sftp request, which
45- # raises a Broken Pipe
46- if e.errno not in (errno.EPIPE,):
47- raise
48+ except _ssh_connection_errors, e:
49 self._raise_connection_error(host, port=port, orig_error=e)
50
51 def connect_ssh(self, username, password, host, port, command):
52@@ -390,14 +382,7 @@
53 argv = self._get_vendor_specific_argv(username, host, port,
54 command=command)
55 return self._connect(argv)
56- except (EOFError), e:
57- self._raise_connection_error(host, port=port, orig_error=e)
58- except (OSError, IOError, socket.error), e:
59- # If the machine is fast enough, ssh can actually exit
60- # before we try and send it the sftp request, which
61- # raises a Broken Pipe
62- if e.errno not in (errno.EPIPE,):
63- raise
64+ except _ssh_connection_errors, e:
65 self._raise_connection_error(host, port=port, orig_error=e)
66
67 def _get_vendor_specific_argv(self, username, host, port, subsystem=None,