Bundle deployment views.

Bundle deployment views.

This branch includes the implementation of the
bundle views. Their goal is to handle the bundle
deployment request/response process.

Views interact with the Deployer instance 
(not yet implemented) in order to schedule,
run and observe bundle deployments.

https://code.launchpad.net/~frankban/charms/precise/juju-gui/guiserver-views/+merge/181020

(do not edit description out of merge proposal)

[frankban, 2013-08-20 11:13:31]

Please take a look.

[bac, 2013-08-20 13:24:09]

LGTM.  Nice tests and well-organized helpers / factoring.

https://codereview.appspot.com/12927049/diff/1/server/guiserver/bundles/utils.py
File server/guiserver/bundles/utils.py (right):

https://codereview.appspot.com/12927049/diff/1/server/guiserver/bundles/utils...
server/guiserver/bundles/utils.py:36: raise response(error='unauthorized access:
unknown user')
The error message is a little misleading.  "unknown user" implies we have a user
but don't recognize her when in fact there is no logged in user.

How about:

error='unauthorized access:  no user logged in.' ?

https://codereview.appspot.com/12927049/diff/1/server/guiserver/bundles/views.py
File server/guiserver/bundles/views.py (right):

https://codereview.appspot.com/12927049/diff/1/server/guiserver/bundles/views...
server/guiserver/bundles/views.py:37: creating this kind of responses:
s/creating this kind of responses/create these responses

https://codereview.appspot.com/12927049/diff/1/server/guiserver/bundles/views...
server/guiserver/bundles/views.py:173: raise response(error='invalid request:
invalid data parameters')
Perhaps list the params in the error message?

[benji, 2013-08-20 13:28:09]

Very nice: LGTM

I only had a small question and a couple of minor word choice suggestions.

https://codereview.appspot.com/12927049/diff/1/server/guiserver/bundles/views.py
File server/guiserver/bundles/views.py (right):

https://codereview.appspot.com/12927049/diff/1/server/guiserver/bundles/views...
server/guiserver/bundles/views.py:60: (the latter will be eventually fixed
switching to a newer version of Python).
I was wondering about why we didn't use Python 3 for this server.  Is there a
Tornado requirement for Python 2?

https://codereview.appspot.com/12927049/diff/1/server/guiserver/bundles/views...
server/guiserver/bundles/views.py:148: deployment being observed. If unseen
changes are available, a response is
"unsent" might be slightly clearer than "unseen".

https://codereview.appspot.com/12927049/diff/1/server/guiserver/bundles/views...
server/guiserver/bundles/views.py:149: suddenly returned containing the changes.
Otherwise, this views suspends
I think "immediately" would be a bit better than "suddenly".

[frankban, 2013-08-20 14:16:42]

*** Submitted:

Bundle deployment views.

This branch includes the implementation of the
bundle views. Their goal is to handle the bundle
deployment request/response process.

Views interact with the Deployer instance 
(not yet implemented) in order to schedule,
run and observe bundle deployments.

R=bac, benji
CC=
https://codereview.appspot.com/12927049

https://codereview.appspot.com/12927049/diff/1/server/guiserver/bundles/utils.py
File server/guiserver/bundles/utils.py (right):

https://codereview.appspot.com/12927049/diff/1/server/guiserver/bundles/utils...
server/guiserver/bundles/utils.py:36: raise response(error='unauthorized access:
unknown user')
On 2013/08/20 13:24:09, bac wrote:
> The error message is a little misleading.  "unknown user" implies we have a
user
> but don't recognize her when in fact there is no logged in user.
> 
> How about:
> 
> error='unauthorized access:  no user logged in.' ?

You are right, changed the message.

https://codereview.appspot.com/12927049/diff/1/server/guiserver/bundles/views.py
File server/guiserver/bundles/views.py (right):

https://codereview.appspot.com/12927049/diff/1/server/guiserver/bundles/views...
server/guiserver/bundles/views.py:37: creating this kind of responses:
On 2013/08/20 13:24:09, bac wrote:
> s/creating this kind of responses/create these responses

Done.

https://codereview.appspot.com/12927049/diff/1/server/guiserver/bundles/views...
server/guiserver/bundles/views.py:60: (the latter will be eventually fixed
switching to a newer version of Python).
On 2013/08/20 13:28:10, benji wrote:
> I was wondering about why we didn't use Python 3 for this server.  Is there a
> Tornado requirement for Python 2?

No, Tornado is fully compatible with Python3. And I guess it should be quite
easy to port the GUI server to Python3. The problem here is that juju-deployer
only supports Python2, and therefore, for now, we cannot upgrade.

https://codereview.appspot.com/12927049/diff/1/server/guiserver/bundles/views...
server/guiserver/bundles/views.py:148: deployment being observed. If unseen
changes are available, a response is
On 2013/08/20 13:28:10, benji wrote:
> "unsent" might be slightly clearer than "unseen".

Done.

https://codereview.appspot.com/12927049/diff/1/server/guiserver/bundles/views...
server/guiserver/bundles/views.py:149: suddenly returned containing the changes.
Otherwise, this views suspends
On 2013/08/20 13:28:10, benji wrote:
> I think "immediately" would be a bit better than "suddenly".

Done.

https://codereview.appspot.com/12927049/diff/1/server/guiserver/bundles/views...
server/guiserver/bundles/views.py:173: raise response(error='invalid request:
invalid data parameters')
On 2013/08/20 13:24:09, bac wrote:
> Perhaps list the params in the error message?

Done.

[frankban, 2013-08-20 14:28:14]

Thank you both for the reviews.