commit 6673109c8bf0da40123e53be3d1b37696ee503ab Author: Isis Lovecruft isis@torproject.org Date: Sun Jan 5 17:33:08 2014 +0000

Cleanup logging of memory addresses in prepopulateRings().

The logging before was quite messy.

The sub hashrings are identified by the HMAC of a unique string. It turns out that the "unique string" which is used in this case is actually something which looks like the class.__repr__() for a bridgedb.Bridges.BridgeHolder subclass. This is *probably* unique because it contains memory addresses of the newly generated hmac_fn()s, which are likely (though not necessarily) unique. This is a bit gross, in my opinion, but it's not really a problem.

However, the log messages this function generated are a problem. They contain memory addresses, which isn't the greatest idea, ignoring all other reasons, simply because it's illegible.

* REFACTOR logging statements in bridgedb.Bridges.FilteredBridgeSplitter.prepopulateRings() to remove logging of hmac_fn() memory addresses. * ADD a long comment detailing the "unique strings" which identify a sub hashring, and how they should probably be changed someday. --- lib/bridgedb/Bridges.py | 41 ++++++++++++++++++++++++++++++++++------- 1 file changed, 34 insertions(+), 7 deletions(-)

diff --git a/lib/bridgedb/Bridges.py b/lib/bridgedb/Bridges.py index c59d5ee..5b07141 100644 --- a/lib/bridgedb/Bridges.py +++ b/lib/bridgedb/Bridges.py @@ -1004,30 +1004,57 @@ class FilteredBridgeSplitter(BridgeHolder): :returns: False if there was a problem adding the subring, True otherwise. """ + # XXX I think subring and ringname are switched in this function, or + # at least that whatever is passed into this function as as the + # `ringname` parameter from somewhere else is odd; for example, with + # the original code, which was `log.debug("Inserted %d bridges into + # hashring '%s'!" % (inserted, ringname))`, this log message appears: + # + # Jan 04 23:18:37 [INFO] Inserted 12 bridges into hashring + # frozenset([<function filterBridgesByIP4 at 0x2d67cf8>, <function + # filterAssignBridgesToRing(<function hmac_fn at 0x3778398>, 4, 0) at + # 0x37de578>])! + # + # I suppose since it contains memory addresses, it *is* technically + # likely to be a unique string, but it is messy. + + hashringName = self.__class__.__name__ + subringName = subring.__class__ + filterNames = [] + if not isinstance(subring, BridgeHolder): - logging.fatal("Can't add '%s' to %s because '%s' isn't a hashring." - % (ringname, self.__class__, ringname)) + logging.fatal("Can't add %r to %s because %r isn't a hashring." + % (ringname, hashringName, ringname)) return False if ringname in self.filterRings.keys(): logging.fatal("Hashring %s already has a subring named '%s'!" % (self.__class__, ringname)) return False

- logging.debug("Adding subring '%s' to hashring %s..." - % (ringname, self.__class__)) + for filterName in [x.func_name for x in list(ringname)]: + # Using `filterAssignBridgesToRing.func_name` gives us a messy + # string which includes all parameters and memory addresses. Get + # rid of this by partitioning at the first `(`: + realFilterName = filterName.partition('(')[0] + filterNames.append(realFilterName) + filterNames = ' '.join(filterNames) + + logging.debug("Adding subring to %s hashring:" % hashringName) + logging.debug(" Subring class: %r" % subringName) + logging.debug(" Subring filters: %s" % filterNames)

#TODO: drop LRU ring if len(self.filterRings) > self.max_cached_rings self.filterRings[ringname] = (filterFn, subring)

if populate_from: - logging.info("Populating hashring %s..." % ringname) + logging.info("Populating hashring %s..." % subringName) inserted = 0 for bridge in populate_from: if isinstance(bridge, Bridge) and filterFn(bridge): subring.insert(bridge) inserted += 1 - logging.info("Inserted %d bridges into hashring %s!" - % (inserted, ringname)) + logging.info("Inserted %d bridges into hashring with filters: %s" + % (inserted, filterNames))

return True