[Mailman-Developers] Mysql MemberAdaptor 1.61 and Mailman 2.1.6

[Adrian Wells]

Mark Sapiro <msapiro at value.net> on Wednesday, October 26, 2005 at 9:06 PM
+0000 wrote:
>Adrian Wells wrote:
>
>>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


So, if I understand this, ideally, Bouncer.py should not be changed to
include additional calls to setBounceInfo(), right?  I'm still trying to
understand whether this is a hack to allow MysqlMemberships.py to work as
is (more or less) OR if setBounceInfo() calls were original "missing" in
Bouncer.py.  I gather the answer is the former.

I have done some searching in the mailman-developers' archives to try to
understand why it was decided to separate BounceInfo.  Here are some
findings:

<http://www.mail-archive.com/mailman-developers@python.org/msg06790.html>:
"I'm putting the "info" parameter from setBounceInfo directly into the
database, which I think is an array itself, not a single value, and the
above doesn't look like Python's just traversing an array, and dumping it
into the database(the LHS names don't tie up with what I think are the
keys for the subelements of "info"), so it looks like I'll have to take a
"best guess" at how to implement this."
>

<http://www.mail-archive.com/mailman-developers@python.org/msg06806.html>:
"...the only changes of any import that I've made are that the Member data
structures are stored in a way that fits MySQL and converted as they are
loaded to the way that fits Mailman, which you'd expect..."

I surmise that the rationale for storing the BounceInfo in separate
columns is to provide easier access via SQL queries to the information
that would otherwise be stored in this object.  I can imagine where this
would be desirable (e.g. quickly querying which members recently received
an increased bounce score).
>
>
>>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.


Agreed.  Fewer lines is preferred.  I apologize for not recognizing what
you had done there.
>
>
>>>>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 at somedomain.net
>>>>bounce score: 1.0
>>>>Oct 25 11:06:54 2005 (2687) falseaddresstest at 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?
>
>
>Yes, I think so, but I'd be inclined to wait a bit and see if there are
>more comments from the list.


I have no problem waiting.
>
>
>>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.


<http://www.mail-archive.com/mailman-developers@python.org/msg06808.html>:
"My suggestion would be to pickle the BounceInfo object on the way into
the database, and unpickle it on the way out."

Or pickle and unpickle this information, right?  Making this change, of
course, will require more effort to extract information stored in MySQL
for other purposes (e.g. a custom web interface) but if it's the best way
to handle this information then I would consider making these changes.  I
will try like to discuss this with the original author of
MysqlMemberships.py.

-Adrian