Code review comment for lp:~gary/charms/precise/juju-gui/authtoken1

Revision history for this message
Francesco Banconi (frankban) wrote :

Nice branch Gary. Tests pass and are very nice.
LGTM with only minors/optional changes.
Thank you!

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

https://codereview.appspot.com/31020043/diff/1/server/guiserver/auth.py#newcode236
server/guiserver/auth.py:236: Here is an example of a token creation
response. Lifetime is in seconds.
Does "Lifetime is in seconds" still apply?

https://codereview.appspot.com/31020043/diff/1/server/guiserver/auth.py#newcode325
server/guiserver/auth.py:325: """Does data represents a token creation
request? True or False."""
Does data represent a token authentication request?

https://codereview.appspot.com/31020043/diff/1/server/guiserver/tests/test_auth.py
File server/guiserver/tests/test_auth.py (right):

https://codereview.appspot.com/31020043/diff/1/server/guiserver/tests/test_auth.py#newcode325
server/guiserver/tests/test_auth.py:325:
self.io_loop.add_timeout.call_args[0][1]()
Nice! It took me a minute before understanding you are calling the
expire_token() function. Maybe we could retrieve the function and then
call it, or just add a comment?

https://codereview.appspot.com/31020043/diff/1/server/guiserver/tests/test_auth.py#newcode362
server/guiserver/tests/test_auth.py:362:
self.io_loop.remove_timeout.assert_called_with('handle marker')
assert_called_once_with? Very cheap double check.

https://codereview.appspot.com/31020043/diff/1/server/guiserver/tests/test_auth.py#newcode389
server/guiserver/tests/test_auth.py:389: # This is a small integration
test of the two functions' interaction.
Very nice!

https://codereview.appspot.com/31020043/

« Back to merge proposal