Code review comment for lp:~tribaal/landscape-client/ceph-usage-report

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

Looks good! +1

A few nitpicks:

#1:
+ ring_id = self._ceph_ring_id
+ if ring_id is None:
+ ring_id = self._get_ceph_ring_id()
+ self._ceph_ring_id = ring_id

I think you can write this as

+ if self._ceph_ring_id is None:
+ self._ceph_ring_id = self._get_ceph_ring_id()

#2:
+ self.assertNotEqual([], plugin._ceph_usage_points)
+ self.assertEqual([(300, 1.0), (600, 1.0)], plugin._ceph_usage_points)

The first assert is redundant.

#3:
+ if len(message["ceph-usages"]) and message["ring-id"] is not None:

len() can be dropped.

review: Approve

« Back to merge proposal