On Wed, Jun 30, 2010 at 16:55, Manuel Franceschini <[email protected]> wrote:
> On Wed, Jun 30, 2010 at 3:08 PM, Iustin Pop <[email protected]> wrote:
> [...]
>>> +  def _SetPeersAddressFamily(self):
>>> +    if not self._peers:
>>> +      raise errors.ConfdClientError("Peer list empty")
>>> +    try:
>>> +      peer = self._peers[0]
>>> +      self._family = utils.GetAddressFamily(peer)
>>> +      for peer in self._peers[1:]:
>>> +        if utils.GetAddressFamily(peer) != self._family:
>>> +          raise errors.ConfdClientError("Peers must be of same address 
>>> family")
>>
>> This could be simplified, a bit:
>>  families = set(utils.GetAddressFamily(peer) for peer in peers)
>>  if len(families) > 1:
>>    …
>>
>> (eliminates the explicit loop)
>
> I like it. The only think is that the following error message
>
>>+      raise errors.ConfdClientError("Peer address %s invalid" % peer)
>
> does not have access to var 'peer' anymore.

True, hence my "could" :)

Up to you which one you prefer, LGTM either way from me.

iustin

Reply via email to