Code review comment for lp://qastaging/~frankban/juju-quickstart/improve-version-handling

Revision history for this message
Francesco Banconi (frankban) wrote :

*** 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.

https://codereview.appspot.com/72520044/

« Back to merge proposal