In general, for sequences (like strings), use the fact that if they are
empty they are also False, e.g.:
use "if seq" instead of "if len(seq) [> 0]".
https://codereview.appspot.com/36080044/diff/1/quickstart/app.py#newcode109
quickstart/app.py:109: 'ssh-add')
Thinking about this in retrospective, I am not sure we need the user to
have an ssh agent set up. It seems all we need are ssh keys, so IIUC the
return codes you described, a 130 could still be ok for us. What do you
think? Feel free to disagree.
https://codereview.appspot.com/36080044/diff/1/quickstart/app.py#newcode121
quickstart/app.py:121: retcode, _, error =
utils.call('/usr/bin/ssh-keygen',
Here, if the user decides to continue, we just generate ssh keys without
starting the agent. This is ok IMHO, but then we don't want to require
the agent on the next quickstart run.
LGTM with (possible) changes, thank you!
https:/ /codereview. appspot. com/36080044/ diff/1/ quickstart/ app.py
File quickstart/app.py (right):
https:/ /codereview. appspot. com/36080044/ diff/1/ quickstart/ app.py# newcode101 app.py: 101: if len(create_keys) > 0 and keys[0] .lower( ) == 'y':
quickstart/
create_
This can be written just as:
if create_keys.lower() == 'y':
In general, for sequences (like strings), use the fact that if they are
empty they are also False, e.g.:
use "if seq" instead of "if len(seq) [> 0]".
https:/ /codereview. appspot. com/36080044/ diff/1/ quickstart/ app.py# newcode109 app.py: 109: 'ssh-add')
quickstart/
Thinking about this in retrospective, I am not sure we need the user to
have an ssh agent set up. It seems all we need are ssh keys, so IIUC the
return codes you described, a 130 could still be ok for us. What do you
think? Feel free to disagree.
https:/ /codereview. appspot. com/36080044/ diff/1/ quickstart/ app.py# newcode121 app.py: 121: retcode, _, error = '/usr/bin/ ssh-keygen' ,
quickstart/
utils.call(
Here, if the user decides to continue, we just generate ssh keys without
starting the agent. This is ok IMHO, but then we don't want to require
the agent on the next quickstart run.
https:/ /codereview. appspot. com/36080044/ diff/1/ quickstart/ app.py# newcode128 app.py: 128: raise ProgramExit('Error generating ssh key!
quickstart/
{}'.format(error))
All the other quickstart messages are lower cased: we might want to
change this, but for now let's follow that pattern.
https:/ /codereview. appspot. com/36080044/ diff/1/ quickstart/ app.py# newcode129 app.py: 129: print('A new SSH key was generated in key_file) )
quickstart/
{}'.format(
Ditto.
https:/ /codereview. appspot. com/36080044/ diff/1/ quickstart/ tests/test_ app.py tests/test_ app.py (right):
File quickstart/
https:/ /codereview. appspot. com/36080044/ diff/1/ quickstart/ tests/test_ app.py# newcode197 tests/test_ app.py: 197: mock_call. assert_ has_calls( [
quickstart/
assert_called_with? <shrug>
https:/ /codereview. appspot. com/36080044/ diff/1/ quickstart/ tests/test_ app.py# newcode239 tests/test_ app.py: 239: assert_ called_ with('/ usr/bin/ ssh-keygen' , called_ once_with? Cheap double check.
quickstart/
mock_call.
assert_
https:/ /codereview. appspot. com/36080044/