Mark Sapiro <msapiro@value.net> on Wednesday, October 26, 2005 at 3:45 PM +0000 wrote:
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.
Thank you for the reply. I had not looked much at OldStyleMemberships.py as it's replaced by the MysqlMemberships.py MemberAdaptor in MailList.py, and I'm continuing to slowly learn how Mailman operates. It turns out there is a bug with MysqlMemberships.py MemberAdaptor which deals with retrieving bounce info cookie which is externally stored (something I learned after reading your helpful comments). I've included the patch for MysqlMemberships.py MemberAdaptor at the end of this message.
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.
"The new kid on the block"... this sounds like a bit of an understatement but I'll have to try to take your word for it. I'm not sure the best way to 'fix' this either hence the initial post to this list.
What is happening is this.
[ snipped helpful and detailed explanation ] Thank you for the helpful and detailed explanation.
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.
OK. I imagine that not much can easily done to change this.
[ snipped my first patch attempt and comments about it]
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.
Thank you for looking over the patch and for providing a more complete patch. Today, I also found the additional sections in which bounce info is changed (as covered by your patch). However, I think there's a couple additional sections that the supplied patch misses - those are when the bounce information is reset (info.reset()). So I've included another patch at the end of this message which seems to be even more complete.
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.
OK. Should this be entered as a bug on SF?
Here's my patch - watch out for wrapped lines.
[ snipped Mark's more complete patch for the sake of brevity (or at least an attempt at it) ] Here's the possibly even more complete patch (as you noted earlier, watch for wrapped lines) for Bouncer.py: --- Bouncer.py.10.25.2005 2005-10-25 12:21:57.000000000 -0400 +++ Bouncer.py 2005-10-26 17:28:46.000000000 -0400 @@ -137,6 +137,10 @@ 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) + # 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) syslog('bounce', '%s: %s has stale bounce info, resetting', self.internal_name(), member) else: @@ -144,6 +148,10 @@ # score and take any necessary action. info.score += weight info.date = day + # 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) syslog('bounce', '%s: %s current bounce score: %s', member, self.internal_name(), info.score) # Continue to the check phase below @@ -158,6 +166,10 @@ self.bounce_score_threshold) self.sendProbe(member, msg) info.reset(0, info.date, info.noticesleft) + # 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) else: self.disableBouncingMember(member, info, msg) @@ -166,6 +178,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 +286,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 This is the patch for the MysqlMemberships.py MemberAdaptor (note this patch was generated against an already patched/modified version of the Mysql MemberAdaptor 1.61): --- MysqlMemberships.py.10.26.2005 2005-10-26 15:07:54.000000000 -0400 +++ MysqlMemberships.py 2005-10-26 15:11:48.000000000 -0400 @@ -499,7 +499,8 @@ DAYOFMONTH(bi_lastnotice), YEAR(bi_date), MONTH(bi_date), - DAYOFMONTH(bi_date) + DAYOFMONTH(bi_date), + bi_cookie FROM mailman_mysql WHERE listname='%s' AND address = '%s'""" @@ -513,7 +514,8 @@ DAYOFMONTH(bi_lastnotice), YEAR(bi_date), MONTH(bi_date), - DAYOFMONTH(bi_date) + DAYOFMONTH(bi_date), + bi_cookie FROM %s WHERE address = '%s'""" %( self.__mlist.internal_name(), MySQLdb.escape_string(member) ) ) @@ -528,6 +530,7 @@ # Otherwise, populate a bounce_info structure. bounce_info = _BounceInfo(member, row[0], (row[5],row[6],row[7]), row[1]) bounce_info.lastnotice = (row[2],row[3],row[4]) + bounce_info.cookie = row[8] return bounce_info # -Adrian