commit 6673109c8bf0da40123e53be3d1b37696ee503ab
Author: Isis Lovecruft <isis(a)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