Code review comment for lp:~fwereade/pyjuju/agent-trash-old-sessions

Revision history for this message
Benjamin Saller (bcsaller) wrote :

LGTM + 1

A few minors

[0] test_session_file_permissions checks the permissions of the file but doesn't seem to handle/test the case where the configured path for the session file isn't writeable. test_nonexistent_directory and test_bad_session_file seem to get at this but it seems to be the check once rather than on each iteration of client. It might be that we want to be able to handle changes to fs perms during the lifetime of the agent.

[1] aggressive use of newline
        + self.assertRaises(
 + JujuError,
 + agent.configure,
 + data)
 +

review: Approve

« Back to merge proposal