[ovs-dev] [PATCH v2 3/5] python: Convert dict iterators.

[Ben Pfaff]

On Wed, Jan 20, 2016 at 05:04:24PM -0500, Russell Bryant wrote:
> On 01/20/2016 04:25 PM, Ben Pfaff wrote:
> > On Tue, Jan 12, 2016 at 02:45:47PM -0500, Russell Bryant wrote:
> >> In Python 2, dict.items(), dict.keys(), and dict.values() returned a
> >> list.  dict.iteritems(), dict.iterkeys(), and dict.itervalues() returned
> >> an iterator.
> >>
> >> As of Python 3, dict.iteritems(), dict.itervalues(), and dict.iterkeys()
> >> are gone.  items(), keys(), and values() now return an iterator.
> >>
> >> In the case where we want an iterator, we now use the six.iter*()
> >> helpers.  If we want a list, we explicitly create a list from the
> >> iterator.
> >>
> >> Signed-off-by: Russell Bryant <russell at ovn.org>
> > 
> > Here, I wonder whether it's safe (before or after the patch) to iterate
> > through 'interfaces' while we're modifying it; some hash tables wouldn't
> > respond well to that:
> 
> It is not safe to modify a list or dict while iterating it, but it's not
> obvious to me that it's happening in this code.  Can you hit me with a
> dash of clue?
> 
> >>  def update_ipsec(ipsec, interfaces, new_interfaces):
> >> -    for name, vals in interfaces.iteritems():
> >> +    for name, vals in six.iteritems(interfaces):
> >>          if name not in new_interfaces:
> >>              ipsec.del_entry(vals["local_ip"], vals["remote_ip"])

Never mind.  The code above is deleting interfaces that are in
'interfaces' but not in 'new_interfaces', but it isn't actually removing
elements from 'interfaces'.

> >>  def get_ssl_cert(data):
> >> -    for ovs_rec in data["Open_vSwitch"].rows.itervalues():
> >> +    for ovs_rec in six.itervalues(data["Open_vSwitch"].rows):
> >>          if ovs_rec.ssl:
> >>              ssl = ovs_rec.ssl[0]
> >>              if ssl.certificate and ssl.private_key:
> > 
> 
> > Here, it seems like there ought to be better ways to get the first value
> > out of an iterator than to convert the whole thing to a list and then
> > take the first element:
> 
> FWIW, that's what the previous code was doing, just implicitly.

That wasn't obvious to me, but I see that now.  Oops (this is my code I
think).

> We can do better, though.  See incremental diff at the end of the message.

Thanks.

> Incremental so far:

Thanks a lot.

Acked-by: Ben Pfaff <blp at ovn.org>