Re: [Mailman-Developers] Mysql MemberAdaptor 1.61 and Mailman 2.1.6

2005-10-26 Thread Mark Sapiro
Adrian Wells wrote:

Adrian Wells [EMAIL PROTECTED] on Monday, October 24, 2005 at 1:56
PM + 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.0 -0400
+++ Bouncer.py  2005-10-25 13:21:02.0 -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: [EMAIL PROTECTED]
bounce score: 1.0
Oct 25 11:06:54 2005 (2687) [EMAIL PROTECTED]: 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 

Re: [Mailman-Developers] Mysql MemberAdaptor 1.61 and Mailman 2.1.6

2005-10-26 Thread Fil

As an aside question I would ask: do you notice speed improvement by
switching to MySQL-based membership? I have a big list of ~180k subscribers
and unfortunately it is now *very* difficult to use the web interface to
unsubscribe people.

Right now I have to resort to command-line instructions and it's not very
practical. I wonder if I should not go the MySQL way, but I'm a bit worried
about taking this risk to my databases (I'd hate to reconstruct 70+ lists
from a backup). Especially if the switch does not bring a solution.

I'd be happy to get advice and maybe even some help if things turn bad, from
people who know this piece of patch.

-- Fil

___
Mailman-Developers mailing list
Mailman-Developers@python.org
http://mail.python.org/mailman/listinfo/mailman-developers
Mailman FAQ: http://www.python.org/cgi-bin/faqw-mm.py
Searchable Archives: http://www.mail-archive.com/mailman-users%40python.org/
Unsubscribe: 
http://mail.python.org/mailman/options/mailman-developers/archive%40jab.org

Security Policy: 
http://www.python.org/cgi-bin/faqw-mm.py?req=showamp;file=faq01.027.htp


Re: [Mailman-Developers] Mysql MemberAdaptor 1.61 and Mailman 2.1.6

2005-10-26 Thread Adrian Wells
Mark Sapiro [EMAIL PROTECTED] on Wednesday, October 26, 2005 at 3:45 PM
+ wrote:
Adrian Wells wrote:

Adrian Wells [EMAIL PROTECTED] on Monday, October 24, 2005 at 1:56
PM + 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: [EMAIL PROTECTED]
bounce score: 1.0
Oct 25 11:06:54 2005 (2687) [EMAIL PROTECTED]: 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.0 -0400
+++ Bouncer.py  2005-10-26 17:28:46.0 -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
+  

Re: [Mailman-Developers] Mysql MemberAdaptor 1.61 and Mailman 2.1.6

2005-10-26 Thread Mark Sapiro
Adrian Wells wrote:

Mark Sapiro [EMAIL PROTECTED] on Wednesday, October 26, 2005 at 3:45 PM
+ wrote:

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.


I think that's right. The MemberAdaptor is not supposed to be in the
business of determining when a _BounceInfo instance has changed behind
its back and divining when to commit changes to it. The documentation
in MemberAdaptor.py says that the getBounceInfo() method returns the
info that was set with setBounceInfo(), so except for what you
discovered about the cookie, MysqlMemberships.py appears to be doing
the right thing at this level.

Actually, it is not really doing the right thing because it is not
supposed to be aware of what's in the _BounceInfo class. The info that
is passed to it is a string representation of the _BounceInfo
instance, and it should really just be saving and retrieving that.
IMO, there should be just one column in the MySQL table for this
string representation. The only possible snag I see is that the string
contains new-lines, and I don't know MySQL so I don't know if
new-lines are allowed in a string field/column.

If MysqlMemberships.py were just storing and retrieving the
representation that it is passed, it wouldn't have to worry about
things like the fact that the 'cookie' argument disappeared from the
_BounceInfo instantiation call in Mailman 2.1.4


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.


Yes. I definitely overlooked the info.reset() two lines before the end
of registerBounce. Good Catch!

However in the earlier part of registerBounce, I deliberately combined
your two calls to setBounceInfo() in the if info is stale clause and
its else clause into a single call following the if - else but still
within the containing else.

I did this even though I think it is logically equivalent, because I
think that all else equal, fewer lines is better.


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: [EMAIL PROTECTED]
bounce score: 1.0
Oct 25 11:06:54 2005 (2687) [EMAIL PROTECTED]: 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?


Yes, I think so, but I'd be inclined to wait a bit and see if there are
more comments from the list.


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):


As I indicate above, I think the better way to fix MysqlMemberships.py
is to remove its knowledge of the _BounceInfo class and just save and
retrieve the string representation that it is handed.

-- 
Mark Sapiro [EMAIL PROTECTED]   The highway is for gamblers,
San Francisco Bay Area, Californiabetter use your sense - B. Dylan

___
Mailman-Developers mailing list
Mailman-Developers@python.org
http://mail.python.org/mailman/listinfo/mailman-developers
Mailman FAQ: http://www.python.org/cgi-bin/faqw-mm.py
Searchable Archives: http://www.mail-archive.com/mailman-users%40python.org/
Unsubscribe: 
http://mail.python.org/mailman/options/mailman-developers/archive%40jab.org

Security Policy: 
http://www.python.org/cgi-bin/faqw-mm.py?req=showamp;file=faq01.027.htp


Re: [Mailman-Developers] Mysql MemberAdaptor 1.61 and Mailman 2.1.6

2005-10-26 Thread John W. Baxter
On 10/26/05 6:06 PM, Mark Sapiro [EMAIL PROTECTED] wrote:

 Actually, it is not really doing the right thing because it is not
 supposed to be aware of what's in the _BounceInfo class. The info that
 is passed to it is a string representation of the _BounceInfo
 instance, and it should really just be saving and retrieving that.
 IMO, there should be just one column in the MySQL table for this
 string representation. The only possible snag I see is that the string
 contains new-lines, and I don't know MySQL so I don't know if
 new-lines are allowed in a string field/column.

Based on these tests dashed off using one of Exim's debugging capabilities
$ exim -be
 ${quote_mysql: A\x0atest}
 A\ntest
 ${quote_mysql: A\x0dtest}
 A\rtest
the newlines are OK but have to be quoted (as do CR characters, and others).

This, of course, assumes that Exim's quote_mysql operator is doing the right
thing.

The best thing would be to check the MySQL documentation (which I'm too lazy
to do this evening).

  --John


___
Mailman-Developers mailing list
Mailman-Developers@python.org
http://mail.python.org/mailman/listinfo/mailman-developers
Mailman FAQ: http://www.python.org/cgi-bin/faqw-mm.py
Searchable Archives: http://www.mail-archive.com/mailman-users%40python.org/
Unsubscribe: 
http://mail.python.org/mailman/options/mailman-developers/archive%40jab.org

Security Policy: 
http://www.python.org/cgi-bin/faqw-mm.py?req=showamp;file=faq01.027.htp