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.
On Tue, Feb 21, 2017 at 09:20:57PM -0000, Newell Jensen wrote: pod_machines( url, headers) pod_machines( url, headers) parameters. get('node_ id') for pm in previous_machines] parameters. get('node_ id') not in previous_node_ids:
> - new_machines = yield self.get_
> - if new_machines == machines:
> + # Sieve the new machine.
> + discovered_machine = None
> + current_machines = yield self.get_
> + previous_node_ids = [
> + pm.power_
> + for cm in current_machines:
> + if cm.power_
> + 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.