Sftp type error #49363

[Aaron Bentley]

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

Robey Pointer wrote:
>      def stop(self):
> -        self.stop_event.set()
> +        self._stop_event.set()
>          # use a timeout here, because if the test fails, the server thread may
>          # never notice the stop_event.
>          self.join(5.0)
> +        self._socket.close()

So threads can't join themseves, but stop isn't mean to be called within
the thread, so it's okay?  This looks correct, but a comment might be
useful.

> +            try:
> +                s, addr_unused = self._socket.accept()
> +                # because the loopback socket is inline, and transports are
> +                # never explicitly closed, best to launch a new thread.
> +                threading.Thread(target=self._callback, args=(s,)).start()
> +            except socket.error, x:
> +                pass #Ignore socket errors

I think socket errors could indicate a serious problem.  Is it necessary
to ignore all of them?

> +            except Exception, x:
> +                # probably a failed test
> +                sys.excepthook(*sys.exc_info())
> +                warning('Exception from within unit test server thread: %r' % x)

This looks overbroad.  Couldn't we get SystemExit?  Wouldn't it be
better to let these exceptions propogate?

>  class SFTPServer(Server):
> @@ -937,10 +947,12 @@
>          """StubServer uses this to log when a new server is created."""
>          self.logs.append(message)
>  
> -    def _run_server(self, s, stop_event):
> +    def _run_server(self, s):
>          ssh_server = paramiko.Transport(s)
>          key_file = os.path.join(self._homedir, 'test_rsa.key')
> -        file(key_file, 'w').write(STUB_SERVER_KEY)
> +        f = open(key_file, 'w')
> +        f.write(STUB_SERVER_KEY)
> +        f.close()

Apparently, this is the intended use: file is the object type and open
is the constructor people should use.  Should we start using this style?

The concept looks okay, but I'd like to understand the exception
swallowing better before voting.

Aaron
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.1 (GNU/Linux)
Comment: Using GnuPG with Thunderbird - http://enigmail.mozdev.org

iD8DBQFEkzlO0F+nu1YWqI0RAoTJAJsFwDW28xxxtVxlAJzyNH4xQm50KACeKUlA
L0jr6mfdd1jc5SUmw0xyXms=
=TGI0
-----END PGP SIGNATURE-----