Code review comment for lp:~stevenk/launchpad/bulk-eid-to-obj

Revision history for this message
William Grant (wgrant) wrote :

20 + eids = []
21 + for entry in logs['log-entries']:
22 + eids.extend([entry['actor'], entry['object']])

This should probably be a set; there will likely be many dupes.

76 +def enterpriseids_to_objects(eids):
77 + """Given a list of SOA Enterprise IDs, return a dict that maps the ID to
78 + its concrete object."""

That's a prolix and non-compliant docstring. "Dereference multiple SOA Enterprise IDs.", maybe?

79 + map_id_to_obj = {}
80 + obj_id_to_eid = defaultdict(dict)

Aren't those "eid_to_obj" and "dbid_to_eid"? map_id_to_obj also isn't needed until right at the end; I'd define it there for readability.

89 + type_ids[scheme[1]].append(int(scheme[2]))
90 + obj_id_to_eid[scheme[1]][int(scheme[2])] = eid

These are redundant. type_ids[foo] is just obj_id_to_eid[foo].keys(). Dropping type_ids also has the side-effect of eliminating dupes.

92 + for kind in types:

This could just iterate over obj_id_to_eid instead, to get just the types relevant to this query.

137 + def test_enterpriseids_to_objects(self):

This probably wants to also test for multiple classes.

review: Needs Fixing (code)

« Back to merge proposal