Code review comment for lp:~frankban/charms/precise/juju-gui/guiserver-views

Revision history for this message
Brad Crittenden (bac) wrote :

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.py#newcode36
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.py#newcode37
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.py#newcode173
server/guiserver/bundles/views.py:173: raise response(error='invalid
request: invalid data parameters')
Perhaps list the params in the error message?

https://codereview.appspot.com/12927049/

« Back to merge proposal