SSH code reorganization.

SSH code reorganization.

This branch introduces a new ssh module
containing SSH handling related code.

The SSH functions previously in app are now
library functions in the ssh module.

ensure_ssh_keys is now the only app entry point
for the whole SSH keys management logic.

For the reasons above this branch mostly includes
code movements + some docstring/comments changes +
some exception handling changes.

https://code.launchpad.net/~frankban/juju-quickstart/improve-version-handling/+merge/210210

(do not edit description out of merge proposal)

[frankban, 2014-03-10 13:53:24]

Please take a look.

[jeff.pihach, 2014-03-10 14:06:43]

LGTM Thanks for this cleanup. I only have one code re-org suggestion if you
choose.

https://codereview.appspot.com/72520044/diff/1/quickstart/app.py
File quickstart/app.py (right):

https://codereview.appspot.com/72520044/diff/1/quickstart/app.py#newcode106
quickstart/app.py:106: ssh.start_agent()
I feel like this should be moved into the check_keys function so the app only
needs to call check_keys once

[bac, 2014-03-10 14:20:16]

LGTM -- thanks for the re-org.

https://codereview.appspot.com/72520044/diff/1/quickstart/app.py
File quickstart/app.py (right):

https://codereview.appspot.com/72520044/diff/1/quickstart/app.py#newcode123
quickstart/app.py:123: 'or cancel [C]? ').lower()
Why tell the user to use upper case C if you're going to lower it?

https://codereview.appspot.com/72520044/diff/1/quickstart/ssh.py
File quickstart/ssh.py (right):

https://codereview.appspot.com/72520044/diff/1/quickstart/ssh.py#newcode73
quickstart/ssh.py:73: key_file = os.path.join(os.path.expanduser('~'), '.ssh',
'id_rsa')
Is it safe to use 'id_rsa'?  I haven't followed the call sites but are we sure
these keys don't already exist?  I'd hate to see us overwrite a user's keys. 
Would it not be better to use something novel like 'id_rsa_quickstart'?

[frankban, 2014-03-10 14:42:32]

*** Submitted:

SSH code reorganization.

This branch introduces a new ssh module
containing SSH handling related code.

The SSH functions previously in app are now
library functions in the ssh module.

ensure_ssh_keys is now the only app entry point
for the whole SSH keys management logic.

For the reasons above this branch mostly includes
code movements + some docstring/comments changes +
some exception handling changes.

R=jeff.pihach, bac
CC=
https://codereview.appspot.com/72520044

https://codereview.appspot.com/72520044/diff/1/quickstart/app.py
File quickstart/app.py (right):

https://codereview.appspot.com/72520044/diff/1/quickstart/app.py#newcode123
quickstart/app.py:123: 'or cancel [C]? ').lower()
On 2014/03/10 14:20:16, bac wrote:
> Why tell the user to use upper case C if you're going to lower it?

Done.

https://codereview.appspot.com/72520044/diff/1/quickstart/ssh.py
File quickstart/ssh.py (right):

https://codereview.appspot.com/72520044/diff/1/quickstart/ssh.py#newcode73
quickstart/ssh.py:73: key_file = os.path.join(os.path.expanduser('~'), '.ssh',
'id_rsa')
On 2014/03/10 14:20:16, bac wrote:
> Is it safe to use 'id_rsa'?  I haven't followed the call sites but are we sure
> these keys don't already exist?  I'd hate to see us overwrite a user's keys. 
> Would it not be better to use something novel like 'id_rsa_quickstart'?

This pre-existed, but IIUC, this is only called if no keys are found (see
check_keys above). For this reason it seems safe to me to create the first one
with the default name.

[frankban, 2014-03-10 15:01:26]

Thank you both!