Adrian Wells wrote:
"Adrian Wells" <adrian@whatifnet.com> on Monday, October 24, 2005 at 1:56 PM +0000 wrote:
I am not proficient in Python and don't completely understand how Mailman operates so I'm interested in finding some help to understand how information generated by registerBounce in Bouncer.py is supposed to reach setBounceInfo in MysqlMemberships.py. Even a general understanding of how bounce information is processed in Mailman would be helpful for investigating this.
After some time and further testing... it seems that the Mysql MemberAdaptor maybe OK after all, but it is not being fully utilized (or any other member adaptor, for that matter)...
I think you are correct about the MysqlMemberships.py MemberAdaptor in particular, but Mailman with OldStyleMemberships.py clearly does record subsequent bounces. See below for more.
The function registerBounce only calls setBounceInfo once (line 116: "self.setBounceInfo(member, info)"). This occurs after testing whether "this is the first bounce we've seen from this member". It would seem as though setBounceInfo should be called a few more times if other conditions are met, right? For example, after determining that the bounce information for a member is valid and is not stale?
I've been looking at this off and on since your first post. I'm kind of "the new kid on the block" here, so even though I think I understand what's going on, I'm not clear on the best way to 'fix' it. What is happening is this. Bouncer.registerBounce calls getBounceInfo to get the bounce info for the member. If there is no bounce info for the member, getBounceInfo returns None and registerBounce creates an instance of the _BounceInfo class and calls setBounceInfo to save it. If there is existing bounce info for the member, getBounceInfo returns the appropriate _BounceInfo class instance which contains the member's info for this list. registerBounce then proceeds to update some attributes of this _BounceInfo instance. Now the tricky part is that in the OldStyleMemberships case, the member _BounceInfo instance is an item in a list of _BounceInfo instances which is the bounce_info attribute of the Mailman list itself. Thus the _BounceInfo instance returned by getBounceInfo is in a sense a pointer into the bounce_info list attribute so when registerBounce changes attributes of the _BounceInfo instance, it is also changing the lists bounce_info attribute so when Save() is ultimately called for the list, the updated bounce info is actually saved. Now, MysqlMemberships.py doesn't work in the same way. its setBounceInfo and getBounceInfo methods take the attributes out of the _BounceInfo instance and store them separately in the database and vice versa, so saving the list doesn't commit any changes that registerBounces may have made to the _BounceInfo instance.
As a result, I've created a patch that seems to correct the unexpected behavior mentioned in my earlier message. This patch may not cover recording when probes occur or how many probes remain (for example in sendNextNotification).
--- Bouncer.py.10.25.2005 2005-10-25 12:21:57.000000000 -0400 +++ Bouncer.py 2005-10-25 13:21:02.000000000 -0400 @@ -137,6 +137,7 @@ if lastbounce + self.bounce_info_stale_after < now: # Information is stale, so simply reset it info.reset(weight, day, self.bounce_you_are_disabled_warnings) + self.setBounceInfo(member, info) syslog('bounce', '%s: %s has stale bounce info, resetting', self.internal_name(), member) else: @@ -144,6 +145,7 @@ # score and take any necessary action. info.score += weight info.date = day + self.setBounceInfo(member, info) syslog('bounce', '%s: %s current bounce score: %s', member, self.internal_name(), info.score) # Continue to the check phase below
Please let me know if this is not good or will otherwise cause problems down the line.
It looks good to me, but as you recognize, it's incomplete. As I said, I'm the new kid on the block. It seems to me that this fix is the right way to go, but others may differ. I've worked up a more complete patch which is pasted to the end of this mail. It addresses the other places where the bounce info is changed. I've also searched for places outside Bouncer.py where bounce info is used, and I think they are all OK as is.
As a minor side note, I noticed the bounce log receives two different formatted messages for the first bounce and subsequent bounces. An example: ... Oct 25 10:50:51 2005 (2687) samplelist: falseaddresstest@somedomain.net bounce score: 1.0 Oct 25 11:06:54 2005 (2687) falseaddresstest@somedomain.net: samplelist current bounce score: 2.0 ... This is not a major issue but it is inconsistent and it not clear why it should be this way. Is there reason is should be different?
I don't think so. All the other log messages from Bouncer are "list: member". I don't see any reason why this one shouldn't also be that way. Here's my patch - watch out for wrapped lines. --- mailman-2.1.6/Mailman/Bouncer.py 2004-12-03 21:01:11.000000000 -0800 +++ mailman-mas/Mailman/Bouncer.py 2005-10-26 12:41:37.984375000 -0700 @@ -146,6 +146,10 @@ info.date = day syslog('bounce', '%s: %s current bounce score: %s', member, self.internal_name(), info.score) + # We've changed info above. In case the MemberAdaptor + # stores bounce info externally to the list, we need + # to tell it to update + self.setBounceInfo(member, info) # Continue to the check phase below # # Now that we've adjusted the bounce score for this bounce, let's @@ -166,6 +170,9 @@ # first bounce, it'll expire by the time we get the disabling bounce. cookie = self.pend_new(Pending.RE_ENABLE, self.internal_name(), member) info.cookie = cookie + # In case the MemberAdaptor stores bounce info externally to + # the list, we need to tell it to save the cookie + self.setBounceInfo(member, info) # Disable them if mm_cfg.VERP_PROBES: syslog('bounce', '%s: %s disabling due to probe bounce received', @@ -271,6 +278,9 @@ msg.send(self) info.noticesleft -= 1 info.lastnotice = time.localtime()[:3] + # In case the MemberAdaptor stores bounce info externally to + # the list, we need to tell it to update + self.setBounceInfo(member, info) def BounceMessage(self, msg, msgdata, e=None): # Bounce a message back to the sender, with an error message if -- Mark Sapiro <msapiro@value.net> The highway is for gamblers, San Francisco Bay Area, California better use your sense - B. Dylan