Wait for the GUI to be ready and started.

Wait for the GUI to be ready and started.

Watch unit changes, open the browser at the
end of the process.

Tests: run `make check`.

QA assuming you have an ec2 Juju env named 'ec2':
- run `.venv/bin/python juju-quickstart -e ec2`.
The environment is bootstrapped, the GUI deployed,
relevant events on the GUI unit notified.
Your default browser shows the GUI when done.

https://code.launchpad.net/~frankban/juju-quickstart/quickstart-watch/+merge/191801

(do not edit description out of merge proposal)

[frankban, 2013-10-18 12:50:20]

Please take a look.

[gary.poster, 2013-10-18 13:11:40]

code LGTM with small comment about comment.  Trying qa now.

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

https://codereview.appspot.com/15020043/diff/1/quickstart/app.py#newcode127
quickstart/app.py:127: def watch(env, service_name):
Nicely readable.

https://codereview.appspot.com/15020043/diff/1/quickstart/manage.py
File quickstart/manage.py (right):

https://codereview.appspot.com/15020043/diff/1/quickstart/manage.py#newcode153
quickstart/manage.py:153: # there is no attached terminal.
I think the looking for an attached terminal is only important if we are trying
to ask the user for input.  I originally thought we would ask the user if he/she
wanted us to open the browser, but in retrospect I think simply defaulting to
opening the browser, and having a flag to disable this behavior, is sufficient. 
And fast. :-)

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

https://codereview.appspot.com/15020043/diff/1/quickstart/tests/test_app.py#n...
quickstart/tests/test_app.py:310: # The glorious moments in the unit's life are
properly highlighted.
LOL

[gary.poster, 2013-10-18 13:27:58]

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

https://codereview.appspot.com/15020043/diff/1/quickstart/app.py#newcode119
quickstart/app.py:119: charm_url = 'cs:precise/juju-gui-77'
Switch to 78?  :-) This should make the deployment a lot faster since 77 is
40-something M and 78 is about 7M, IIRC.

[gary.poster, 2013-10-18 13:45:07]

make check: good.  The obsessive compulsive in me wants to see coverage for the
run function, so we get 100% ;-)

I had some issues in QA but they seemed Juju-related, not quickstart related,
and as you suggested, my system has an old Juju.  I will update and try again.

During the qa, though, the text did confuse me.

---
connecting to wss://ec2-50-17-120-194.compute-1.amazonaws.com:17070
deploying Juju GUI
service deployed and exposed
juju-gui/0 deployment is pending
---

What would you think of changing the second and third lines to something like
these?

---
connecting to wss://ec2-50-17-120-194.compute-1.amazonaws.com:17070
requesting Juju GUI deployment
Juju GUI deployment request accepted
juju-gui/0 deployment is pending
---

Better ideas welcome, but I think it is weird to see "deployed and exposed" but
then "deployment is pending".

[frankban, 2013-10-18 14:07:57]

Please take a look.

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

https://codereview.appspot.com/15020043/diff/1/quickstart/app.py#newcode119
quickstart/app.py:119: charm_url = 'cs:precise/juju-gui-77'
On 2013/10/18 13:27:58, gary.poster wrote:
> Switch to 78?  :-) This should make the deployment a lot faster since 77 is
> 40-something M and 78 is about 7M, IIRC.

Yeah, and it also includes the string encoding fix for the GUI server.

https://codereview.appspot.com/15020043/diff/1/quickstart/manage.py
File quickstart/manage.py (right):

https://codereview.appspot.com/15020043/diff/1/quickstart/manage.py#newcode153
quickstart/manage.py:153: # there is no attached terminal.
On 2013/10/18 13:11:40, gary.poster wrote:
> I think the looking for an attached terminal is only important if we are
trying
> to ask the user for input.  I originally thought we would ask the user if
he/she
> wanted us to open the browser, but in retrospect I think simply defaulting to
> opening the browser, and having a flag to disable this behavior, is
sufficient. 
> And fast. :-)

Cool, changed the XXX comment and the related card description.

[gary.poster, 2013-10-18 14:33:29]

qa good.  Very cool.  Thank you!

[frankban, 2013-10-18 14:40:39]

On 2013/10/18 13:45:07, gary.poster wrote:
> make check: good.  The obsessive compulsive in me wants to see coverage for
the
> run function, so we get 100% ;-)
> 
> I had some issues in QA but they seemed Juju-related, not quickstart related,
> and as you suggested, my system has an old Juju.  I will update and try again.
> 
> During the qa, though, the text did confuse me.
> 
> ---
> connecting to wss://ec2-50-17-120-194.compute-1.amazonaws.com:17070
> deploying Juju GUI
> service deployed and exposed
> juju-gui/0 deployment is pending
> ---
> 
> What would you think of changing the second and third lines to something like
> these?
> 
> ---
> connecting to wss://ec2-50-17-120-194.compute-1.amazonaws.com:17070
> requesting Juju GUI deployment
> Juju GUI deployment request accepted
> juju-gui/0 deployment is pending
> ---
> 
> Better ideas welcome, but I think it is weird to see "deployed and exposed"
but
> then "deployment is pending".

Done, thanks for the review Gary!

[frankban, 2013-10-18 14:41:56]

*** Submitted:

Wait for the GUI to be ready and started.

Watch unit changes, open the browser at the
end of the process.

Tests: run `make check`.

QA assuming you have an ec2 Juju env named 'ec2':
- run `.venv/bin/python juju-quickstart -e ec2`.
The environment is bootstrapped, the GUI deployed,
relevant events on the GUI unit notified.
Your default browser shows the GUI when done.

R=gary.poster
CC=
https://codereview.appspot.com/15020043