Code review comment for lp:~makyo/juju-quickstart/ssh-3-watch-keys

Revision history for this message
Gary Poster (gary) wrote :

What you've added looks good to me, with the changes I've suggested, but
QA bad.

------------------------------------------------
Continue [y/N] or watch for new keys [w]? w
Please run this command in another terminal or window and follow the
instructions it produces; quickstart will continue when keys are
generated. ^C to quit.

ssh-keygen -b 4096 -t rsa

.
.
.
.
.
.
.
.
.
.
.
bootstrapping the ec2 environment (type: ec2)
juju-quickstart: error: ERROR error parsing environment "ec2": no public
ssh keys found
------------------------------------------------

Do we need to start (restart) an agent?

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

https://codereview.appspot.com/39610049/diff/1/quickstart/app.py#newcode112
quickstart/app.py:112: raise ProgramExit('Please run this command and
follow the '
On 2013/12/13 01:49:45, gary.poster wrote:
> If you would like to create the keys yourself, please run this command

, follow its instructions,

> and then
> re-run quickstart:

> ssh-keygen -b 4096 -t rsa

https://codereview.appspot.com/39610049/diff/1/quickstart/app.py#newcode120
quickstart/app.py:120: return utils.call('/usr/bin/find', ssh_dir,
'-name', 'id_*.pub')[0]
On 2013/12/13 01:49:45, gary.poster wrote:
> We can wait for Windows compatibility, but it would be nice if
quickstart used
> built in Python tools rather than find. <shrug> for now.
That is: please leave as is for now unless you find yourself really
motivated and it looks really fast and easy.

https://codereview.appspot.com/39610049/diff/1/quickstart/app.py#newcode130
quickstart/app.py:130: print('.')
I think this would be much better. OK with you?

print('.', end="")

https://codereview.appspot.com/39610049/diff/1/quickstart/tests/test_app.py
File quickstart/tests/test_app.py (right):

https://codereview.appspot.com/39610049/diff/1/quickstart/tests/test_app.py#newcode245
quickstart/tests/test_app.py:245: def test_watch(self, mock_print,
mock_sleep):
Nice test.

https://codereview.appspot.com/39610049/

« Back to merge proposal