Code review comment for lp:~jtv/maas/bug-1116700

Revision history for this message
Raphaƫl Badin (rvb) wrote :

Looks good to me.

Thanks for updating the test's name and adding a new test.

3 tiny notes:

[0]

50 + node.hostname, lease.ip, mac.id
[...]
59 - for hostname, ip in cursor.fetchall()
60 + for hostname, ip, mac in cursor.fetchall()

You probably don't need to return the 'mac' in the query since you're ignoring it later.

[1]

101 + lease_for_newer_mac = factory.make_dhcp_lease(
102 + nodegroup=node.nodegroup, mac=newer_mac.mac_address)
103 + ignore_unused(lease_for_newer_mac)

A bit far fetched :) I'd get rid of the assignment and the ignore_unused() statement because I think the name of the other variable 'lease_for_older_mac' make the whole design pretty clear.

[2]

69 -# Copyright 2012 Canonical Ltd. This software is licensed under the
70 +# Copyright 2013 Canonical Ltd. This software is licensed under the

That's a nice touch but now you got me wondering about why you did not update dhcplease.py's header ;).

review: Approve

« Back to merge proposal