[Mailman-Developers] MysqlMemberships.py [was: Informal "MEP" process, anyone? [was: PHP Wrappers?]]

[Mark Sapiro]

Fil wrote:
>
>Well, it currently has a "works *not* for me" flavor and I'd appreciate som
>help on the two outstanding bugs I still have (in my version, but I guess
>Kev's version has them too).


Yes, Kev's has them too, at least the 1.61 version.


>1) if the DB connection dies or fades away, ping() is never called. I
>suspect this is due to the fact that the function self._prodServerConnection
>is not called with parenthesis. However if I add the parenthesis, I get
>bumped by an error because the function ping() itself does not exist at this
>point.


Well, yes and no. You are correct that the statement

    self._prodServerConnection

has no effect other than to tell a human reader what needs to be done
there. It needs to be

    self._prodServerConnection()

The reason that doesn't work is because ping() is a method of the
MySQLdb.connect class which has been instantiated in
MySqlMemberships.py as 'connection'. Thus, just as for example, to pop
an item off the end of a list L, you need to do

    x = L.pop()

and not

    x = pop(L)

in _prodServerConnection(), you can't do

    if ping(self.connection) == 0:

you need to do

    if self.connection.ping() == 0:



>2) There's an issue with the bounce daemon not properly calling the __init__
>sequence (or so it seems), and I get failures when I activate this daemon.


This is a known (at least to me) issue (and I don't even use
MysqlMemberships.py). I alluded to this in last month's thread on
MysqlMemberships.py in the post at
<http://mail.python.org/pipermail/mailman-developers/2005-October/018264.html>
where I said

>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

More explicitly, in Mailman 2.1.4 the 'cookie' argument was dropped
from the __init__ method of the _BounceInfo. Looking at your code, it
seems that you have adjusted for this. Kev's 1.61 has


    # Otherwise, populate a bounce_info structure.
    bounce_info = _BounceInfo(member, row[1], (row[6],row[7],row[8]),
            row[2], row[0])
    bounce_info.lastnotice = (row[3],row[4],row[5])
    return bounce_info

Yours has

    # 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

which is correct if you rearranged the columns to move the cookie from
column 0 to column 8 and shifted the other columns to the left, and it
looks like you've done that.

There is still a larger issue discussed in last months thread about how
and when to commit the bounce info to the MySQL database since the
'stock' Bouncer.py relies on the fact that it passes a reference to
the _BounceInfo instance which is stored in the list structure so it
can change the _BounceInfo instance between calling setBounceInfo and
saving the list and have those changes saved. This cannot work with
MysqlMemberships.py so Bouncer.py must call setBounceInfo after all
changes are complete.

There is also the issue of how much knowledge of the (private)
_BounceInfo class MysqlMemberships.py should have. Granted, it must
have some knowledge since it can't directly store the instance
reference in the MySQL database, and it is also desirable to have this
info in the database in a more useable form than say a Python pickle,
but this all leads to problems when the _BounceInfo class definition
or its methods change.

So, my cursory examination makes me think your MysqlMemberships.py code
is OK. What are the actual errors that you get from bounce processing?


>In both cases, I'd appreciate feedback, a hint to documentation, plain tests
>or even a full patch :)


HTH

-- 
Mark Sapiro <msapiro at value.net>       The highway is for gamblers,
San Francisco Bay Area, California    better use your sense - B. Dylan