Initial implementation of the Juju GUI server.

Initial implementation of the Juju GUI server.

This first cut of the server replaces the 
functionality provided in the charm by
haproxy and apache.

The server includes:
- static files serving;
- a WebSocket proxy handling the
  connections between the browser and
  the server and between the server and
  the Juju API;
- HTTP to HTTPS redirecting.

Further features like auth and deployer
management will be added in the future.

ws4py is used to implement the WebSocket
client: there is something to fix there and
it will be done in a future branch.

At this time there is no easy way to QA the
server, but tests can be run using
`make unittest`: the test/lint suite is already
integrated with the one used by the charm.

Unfortunately the diff is quite long, sorry!

https://code.launchpad.net/~frankban/charms/precise/juju-gui/server-base/+merge/175624

(do not edit description out of merge proposal)

[frankban, 2013-07-18 16:07:25]

Please take a look.

[gary.poster, 2013-07-18 21:07:31]

Wow, code LGTM.  I'll try to run tests now...

https://codereview.appspot.com/11530043/diff/1/server/guiserver/apps.py
File server/guiserver/apps.py (right):

https://codereview.appspot.com/11530043/diff/1/server/guiserver/apps.py#newco...
server/guiserver/apps.py:32: # Avoid module level import so that options can be
properly set up.
Mm, so tornado has import side effects?  :-/ too bad.

[bac, 2013-07-18 21:52:55]

LGTM: I echo Gary's 'wow' -- good work.  I'll coordinate with Gary and see if I
need to run the tests too.

https://codereview.appspot.com/11530043/diff/1/Operation.md
File Operation.md (right):

https://codereview.appspot.com/11530043/diff/1/Operation.md#newcode22
Operation.md:22: 443. HTTP connections to the 80 port are redirected to the
former one.
change to: connections to port 80 are

https://codereview.appspot.com/11530043/diff/1/server/guiserver/__init__.py
File server/guiserver/__init__.py (right):

https://codereview.appspot.com/11530043/diff/1/server/guiserver/__init__.py#n...
server/guiserver/__init__.py:28: performs the actual orchestration work. Both
browser-server and server- Juju
Super trivial:

Remove the space: server-Juju

https://codereview.appspot.com/11530043/diff/1/server/guiserver/apps.py
File server/guiserver/apps.py (right):

https://codereview.appspot.com/11530043/diff/1/server/guiserver/apps.py#newco...
server/guiserver/apps.py:57: ], debug=options.debug)
I don't understand how this only redirects HTTP - HTTPS.  Why doesn't HTTPS
traffic go to this handler?

https://codereview.appspot.com/11530043/diff/1/server/guiserver/clients.py
File server/guiserver/clients.py (right):

https://codereview.appspot.com/11530043/diff/1/server/guiserver/clients.py#ne...
server/guiserver/clients.py:35: new message is received by the client.
I'd say something about the optional *args and **kwargs.  Not sure what.

https://codereview.appspot.com/11530043/diff/1/server/guiserver/clients.py#ne...
server/guiserver/clients.py:61: # FIXME: find a way to avoid redifining
self.sock here.
typo: redefining

https://codereview.appspot.com/11530043/diff/1/server/guiserver/clients.py#ne...
server/guiserver/clients.py:84: # FIXME: find a way to avoid redifining
self.sock here.
same typo

https://codereview.appspot.com/11530043/diff/1/server/guiserver/clients.py#ne...
server/guiserver/clients.py:98: # FIXME: this seems clearly an error in ws4py.
Maybe explain what you're referring to here.

https://codereview.appspot.com/11530043/diff/1/server/guiserver/manage.py
File server/guiserver/manage.py (right):

https://codereview.appspot.com/11530043/diff/1/server/guiserver/manage.py#new...
server/guiserver/manage.py:78: redirector().listen(80)
Oh, there it is!  NM.

https://codereview.appspot.com/11530043/diff/1/server/guiserver/tests/test_cl...
File server/guiserver/tests/test_clients.py (right):

https://codereview.appspot.com/11530043/diff/1/server/guiserver/tests/test_cl...
server/guiserver/tests/test_clients.py:60: 
Why does this test not use self.received like the next one does?

[frankban, 2013-07-19 10:27:48]

Please take a look.

https://codereview.appspot.com/11530043/diff/1/Operation.md
File Operation.md (right):

https://codereview.appspot.com/11530043/diff/1/Operation.md#newcode22
Operation.md:22: 443. HTTP connections to the 80 port are redirected to the
former one.
On 2013/07/18 21:52:56, bac wrote:
> change to: connections to port 80 are

Done.

https://codereview.appspot.com/11530043/diff/1/server/guiserver/__init__.py
File server/guiserver/__init__.py (right):

https://codereview.appspot.com/11530043/diff/1/server/guiserver/__init__.py#n...
server/guiserver/__init__.py:28: performs the actual orchestration work. Both
browser-server and server- Juju
On 2013/07/18 21:52:56, bac wrote:
> Super trivial:
> 
> Remove the space: server-Juju
> 

Done.

https://codereview.appspot.com/11530043/diff/1/server/guiserver/apps.py
File server/guiserver/apps.py (right):

https://codereview.appspot.com/11530043/diff/1/server/guiserver/apps.py#newco...
server/guiserver/apps.py:32: # Avoid module level import so that options can be
properly set up.
On 2013/07/18 21:07:31, gary.poster wrote:
> Mm, so tornado has import side effects?  :-/ too bad.

No, actually I will replace this with a regulat module level import.
I misinterpreted the doc: "All modules that define options must have been
imported before the command line is parsed."
We do that in manage.setup(), so I guess this can be changed.

https://codereview.appspot.com/11530043/diff/1/server/guiserver/clients.py
File server/guiserver/clients.py (right):

https://codereview.appspot.com/11530043/diff/1/server/guiserver/clients.py#ne...
server/guiserver/clients.py:35: new message is received by the client.
On 2013/07/18 21:52:56, bac wrote:
> I'd say something about the optional *args and **kwargs.  Not sure what.

Done.

https://codereview.appspot.com/11530043/diff/1/server/guiserver/clients.py#ne...
server/guiserver/clients.py:61: # FIXME: find a way to avoid redifining
self.sock here.
On 2013/07/18 21:52:56, bac wrote:
> typo: redefining

Done.

https://codereview.appspot.com/11530043/diff/1/server/guiserver/clients.py#ne...
server/guiserver/clients.py:84: # FIXME: find a way to avoid redifining
self.sock here.
On 2013/07/18 21:52:56, bac wrote:
> same typo

Done.

https://codereview.appspot.com/11530043/diff/1/server/guiserver/clients.py#ne...
server/guiserver/clients.py:98: # FIXME: this seems clearly an error in ws4py.
On 2013/07/18 21:52:56, bac wrote:
> Maybe explain what you're referring to here.

Done.

https://codereview.appspot.com/11530043/diff/1/server/guiserver/tests/test_cl...
File server/guiserver/tests/test_clients.py (right):

https://codereview.appspot.com/11530043/diff/1/server/guiserver/tests/test_cl...
server/guiserver/tests/test_clients.py:60: 
On 2013/07/18 21:52:56, bac wrote:
> Why does this test not use self.received like the next one does?

Here we want to just check that a message can be correctly received by the
client.
In the next test we also check that, when a message is received, a callback
(i.e. self.received.append) is called passing the message.

[frankban, 2013-07-19 10:35:21]

Thank you both for the reviews!

[frankban, 2013-07-19 14:20:48]

*** Submitted:

Initial implementation of the Juju GUI server.

This first cut of the server replaces the 
functionality provided in the charm by
haproxy and apache.

The server includes:
- static files serving;
- a WebSocket proxy handling the
  connections between the browser and
  the server and between the server and
  the Juju API;
- HTTP to HTTPS redirecting.

Further features like auth and deployer
management will be added in the future.

ws4py is used to implement the WebSocket
client: there is something to fix there and
it will be done in a future branch.

At this time there is no easy way to QA the
server, but tests can be run using
`make unittest`: the test/lint suite is already
integrated with the one used by the charm.

Unfortunately the diff is quite long, sorry!

R=gary.poster, bac
CC=
https://codereview.appspot.com/11530043

[hazmat, 2013-11-06 08:43:28]

Just poking through the charm and came across this, its really nice work, well
done.

> R=gary.poster, bac
> CC=
> https://codereview.appspot.com/11530043