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 <russ...@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"]) >> >> - for name, vals in new_interfaces.iteritems(): >> + for name, vals in six.iteritems(new_interfaces): >> orig_vals = interfaces.get(name) >> if orig_vals: >> # Configuration for this host already exists. Check if it's > > Here, we might as well just use plain values() since there can be at > most one record in this table: Sure, that works for me. >> 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. We can do better, though. See incremental diff at the end of the message. >> @@ -398,10 +400,10 @@ class Datum(object): >> def as_scalar(self): >> if len(self.values) == 1: >> if self.type.is_map(): >> - k, v = self.values.iteritems()[0] >> + k, v = list(six.iteritems(self.values))[0] >> return [k.value, v.value] >> else: >> - return self.values.keys()[0].value >> + return list(self.values.keys())[0].value >> else: >> return None > > Same here, in xenserver/usr_share_openvswitch_scripts_ovs-xapi-sync: >> @@ -84,7 +85,7 @@ def get_network_by_bridge(br_name): >> recs = session.xenapi.network.get_all_records_where( >> 'field "bridge"="%s"' % br_name) >> if len(recs) > 0: >> - return recs.values()[0] >> + return list(recs.values())[0] >> >> return None >> Incremental so far: > diff --git a/debian/ovs-monitor-ipsec b/debian/ovs-monitor-ipsec > index 2bccfb0..1502294 100755 > --- a/debian/ovs-monitor-ipsec > +++ b/debian/ovs-monitor-ipsec > @@ -378,7 +378,7 @@ def update_ipsec(ipsec, interfaces, new_interfaces): > > > def get_ssl_cert(data): > - for ovs_rec in six.itervalues(data["Open_vSwitch"].rows): > + for ovs_rec in data["Open_vSwitch"].rows.values(): > if ovs_rec.ssl: > ssl = ovs_rec.ssl[0] > if ssl.certificate and ssl.private_key: > diff --git a/python/ovs/db/data.py b/python/ovs/db/data.py > index cd535d4..3075ee6 100644 > --- a/python/ovs/db/data.py > +++ b/python/ovs/db/data.py > @@ -356,7 +356,7 @@ class Datum(object): > return ["map", [[k.to_json(), v.to_json()] > for k, v in sorted(self.values.items())]] > elif len(self.values) == 1: > - key = list(self.values.keys())[0] > + key = next(six.iterkeys(self.values)) > return key.to_json() > else: > return ["set", [k.to_json() for k in sorted(self.values.keys())]] > @@ -400,10 +400,10 @@ class Datum(object): > def as_scalar(self): > if len(self.values) == 1: > if self.type.is_map(): > - k, v = list(six.iteritems(self.values))[0] > + k, v = next(six.iteritems(self.values)) > return [k.value, v.value] > else: > - return list(self.values.keys())[0].value > + return next(six.iterkeys(self.values)).value > else: > return None > > diff --git a/xenserver/usr_share_openvswitch_scripts_ovs-xapi-sync > b/xenserver/usr_share_openvswitch_scripts_ovs-xapi-sync > index cbc9a92..a776c00 100755 > --- a/xenserver/usr_share_openvswitch_scripts_ovs-xapi-sync > +++ b/xenserver/usr_share_openvswitch_scripts_ovs-xapi-sync > @@ -85,7 +85,7 @@ def get_network_by_bridge(br_name): > recs = session.xenapi.network.get_all_records_where( > 'field "bridge"="%s"' % br_name) > if len(recs) > 0: > - return list(recs.values())[0] > + return next(six.itervalues(recs)) > > return None > -- Russell Bryant _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
