Introduce the beginnings of an ssh app.

Review Request #3405 — Created Oct. 8, 2012 and submitted

Information

Review Board
release-1.6.x

Reviewers

Introduce the beginnings of an ssh app.

This introduces an ssh app, which factors out and structures the SSH
utility functions originally found in scmtools/sshutils.py.

The new ssh app contains an SSHClient object, which represents a session
tied either to a base .ssh directory, or a namespaced directory within.
Down the road, we may want to take further advantage of the namespacing,
such as for repository access. Right now, we deifne a namespace as a
'/'-separated path.

Most of the original utility functions in sshutils now live within
SSHClient. Instead of each of those taking a local_site_name parameter,
this is now passed only when defining the SSHClient, as the namespace.

Multiple clients can exist at a time.

The various SSH-related errors have moved into ssh/errors.py, and have
been decoupled from SCMError. This is clean in all cases except
AuthenticationError. Both apps have a concept of an AuthenticationError,
with ssh's being a bit more specific.

What we're doing is keeping two versions. The SSH one is
SSHAuthenticationError, and contains all the original logic. The new one
is AuthenticationError, and basically has SSHAuthenticationError as a
mixin. The two call sites in scmtools that need to really deal with this
check for an SSHAuthenticationError and convert it into an
AuthenticationError. The purpose of this is to keep the ssh app
decoupled from SCMTools.
All unit tests pass.
Description From Last Updated

Can you combine these?

daviddavid

There's an extra line here

daviddavid

Why did you switch the parens for a continuation character?

daviddavid

I think this would be less confusing if you initialized ssh_dir_name to None and then did the fallback to .ssh …

daviddavid
david
  1. 
      
  2. reviewboard/ssh/client.py (Diff revision 1)
     
     
     
     
     
     
     
     
     
     
    Should this be moved to ssh.errors?
  3. reviewboard/ssh/client.py (Diff revision 1)
     
     
     
     
     
    Show all issues
    Can you combine these?
  4. reviewboard/ssh/errors.py (Diff revision 1)
     
     
    Show all issues
    There's an extra line here
  5. 
      
chipx86
david
  1. 
      
  2. reviewboard/scmtools/tests.py (Diff revision 2)
     
     
     
    Show all issues
    Why did you switch the parens for a continuation character?
    1. Not sure I did it consciously, but I feel like this:
      
      foo = (
          bar and baz)
      
      is weird. The \ feels more natural in this case.
      
      Using the previous form from the old change wouldn't have fit in 80 columns.
      
      If you feel strongly about it, I'll change it.
    2. I  guess it's fine.
  3. reviewboard/ssh/client.py (Diff revision 2)
     
     
     
     
    Show all issues
    I think this would be less confusing if you initialized ssh_dir_name to None and then did the fallback to .ssh after the loop.
    1. Sure. If you don't mind, I'm going to leave this particular change alone, though, because all this moves in my next commit. I'll fix it there.
  4. 
      
david
  1. Ship It!
  2. 
      
chipx86
Review request changed
Status:
Completed
Change Summary:
Pushed to release-1.6.x (575d48f)