Code review comment for lp:~newell-jensen/maas/fix-1665839

Revision history for this message
Christian Reis (kiko) wrote :

On Tue, Feb 21, 2017 at 09:20:57PM -0000, Newell Jensen wrote:
> - new_machines = yield self.get_pod_machines(url, headers)
> - if new_machines == machines:
> + # Sieve the new machine.
> + discovered_machine = None
> + current_machines = yield self.get_pod_machines(url, headers)
> + previous_node_ids = [
> + pm.power_parameters.get('node_id') for pm in previous_machines]
> + for cm in current_machines:
> + if cm.power_parameters.get('node_id') not in previous_node_ids:
> + discovered_machine = cm

Much nicer!

Can you explain why you only care about the first machine you find in
current_machines? This class of code is always a source of weird bugs --
the list might come sorted the way you expect, or it might contain
something unexpected.

If current_machines is always a list of 1 item, then assert() that. And
if not, checking that the list contains what you expect is a good idea.

« Back to merge proposal