Code review comment for lp://staging/~chad.smith/landscape-client/swift-device-monitoring

Revision history for this message
Alberto Donato (ack) wrote :

Looks good! +1

Just minor comments:

#1:
+ if content is None:
+ return None

Maybe it's not a real case, but an empty string as content would pass this and make json.loads() fail after, it'd be safer to just use "if not content".

#2:
+ def test_get_swift_devices_no_swift_python_libs_available(self):

This test assumes swift libraries are not installed on the machine.

#3:
Please call tests that exercise private methods with the test_wb_ prefix.

review: Approve

« Back to merge proposal