Mailman 3.0 support for Postgres

Bug #860159 reported by Stephen A. Goss
8
This bug affects 1 person
Affects Status Importance Assigned to Milestone
GNU Mailman
Fix Released
High
Barry Warsaw

Bug Description

Currently, the MM3 source is SQLite specific, but only in the schema and database creation code, otherwise, it uses Storm which (mostly) supports Postgres.

Tags: mailman3

Related branches

Revision history for this message
Stephen A. Goss (postfuturist) wrote :

I've attached a branch with the Postgres support code that I've cooked up.

Some notes:

1. You must install Storm trunk (or v. 0.19 whenever that is released) as version 0.18 is broken w/regards to Postgres and UUID columns.

2. I haven't adapted the test suite to run against Postgres, yet, but my own external integration tests pass (which encompass quite a bit of functionality).

3. There is now an alternate mailman_pg.sql file which is used to create the tables. Currently, two foreign key constraints are commented out because those are violated in Mailman 3 (apparently this doesn't bother SQLite). Some column TYPES are different from mailman.sql. Classes are created in a slightly different order due to FK constraint creation requires the referenced table to actually exist. The primary key indexes defined after each class are probably redundant, as those are created automatically for SERIAL columns defined as PRIMARY KEY in Postgres.

4. Probably more FK constraint violations exist that my tests haven't uncovered.

Revision history for this message
Barry Warsaw (barry) wrote : Re: [Bug 860159] Re: Mailman 3.0 support for Postgres

On Sep 27, 2011, at 12:44 AM, Stephen A. Goss wrote:

>I've attached a branch with the Postgres support code that I've cooked
>up.

Fantastic, thanks! Do be sure to add a merge proposal.

>3. There is now an alternate mailman_pg.sql file which is used to create
>the tables. Currently, two foreign key constraints are commented out
>because those are violated in Mailman 3 (apparently this doesn't bother
>SQLite). Some column TYPES are different from mailman.sql. Classes are
>created in a slightly different order due to FK constraint creation
>requires the referenced table to actually exist. The primary key indexes
>defined after each class are probably redundant, as those are created
>automatically for SERIAL columns defined as PRIMARY KEY in Postgres.

Do you have any thoughts on whether the two .sql files can possibly be shared?
My biggest concern is that it will be difficult-ish to keep them in sync as I
add or modify the SQLite version. If it's not possible, so be it.

I took a quick look at the Python changes, and I think I'm going to refactor
the code to not hardcode so much in stock.py. For example, I'll probably
rename StockDatabase to SQLiteDatabase and add a PostgresDatabase class,
adding a common super class. That way, you'd only need to put this in your
mailman.cfg file:

[database]
class: mailman.database.postgres.PostgresDatabase

Don't worry about that too much, I can make that change when I merge your
branch.

>4. Probably more FK constraint violations exist that my tests haven't
>uncovered.

I'd definitely like to be able to run the test suite against Postgres, if even
for now it's a manual select (e.g. because Postgres would obviously have to be
installed and configured in order to work).

Revision history for this message
Stephen A. Goss (postfuturist) wrote :

The two SQL files really can't be shared due to the differences in those database engines, and even less so if anyone wants to bother to add MySQL support (inevitable, I imagine).

We could write the database schema in some simplified intermediate form and generate the appropriate SQL. If we were using Django's ORM or SQLAlchemy, we would generate that directly from the model class definitions in code.

Barry Warsaw (barry)
Changed in mailman:
status: New → Confirmed
importance: Undecided → High
assignee: nobody → Barry Warsaw (barry)
milestone: none → 3.0.0b1
Barry Warsaw (barry)
Changed in mailman:
status: Confirmed → In Progress
Revision history for this message
Barry Warsaw (barry) wrote :

I'm going to accept the patch and merge it into trunk, with some refactoring to separate out the SQLite and PostgreSQL database. You will need to change the `class` key in the [database] section of your mailman.cfg to enable PostgreSQL.

We still need documentation, and if possible tests against Postgres.

Revision history for this message
Barry Warsaw (barry) wrote :

I've had to comment out a few more constraints to get the suite to run without failure. I do still get 7 test failures with the branch though. I'll push what I have and will continue to work on getting the test suite to pass. I won't merge the branch until I do. Then I'll file a separate bug on the constraint issues.

Revision history for this message
Barry Warsaw (barry) wrote :

I'm down to three test failures in the above branch. There seems to be two actual bugs going on here:

1) Even though I fixed the SQL type for digest_size_threshold to REAL for Postgres (and FLOAT for SQLite, but it doesn't care), it still seems that when we store 10.5 into it in configuration.rst, 11.0 comes out.

2) It looks to me like Postgres doesn't reset the primary key id to 1 for deleted tables. Two of the failing tests print the Pended dictionary, which contains its id, i.e. its primary key. For SQLite, this always gets reset to 1 for subsequent tests (because we delete all tables between every test). For Postgres though, deleting the table does not seem to reset the primary key back to 1.

This might be a case where the tests need to be fixed to not assume the id is already reset at the beginning of the test, unless there's a good way to guarantee this for Postgres.

Revision history for this message
Barry Warsaw (barry) wrote :

Never mind. I've fixed these all and now have the full test suite working with PostgreSQL. I'll write up a little documentation and merge this to trunk. Thanks for the great work!

Barry Warsaw (barry)
Changed in mailman:
status: In Progress → Fix Committed
Revision history for this message
Stephane Wirtel (OpenERP) (stephane-openerp) wrote :

Hi all,

I'm interested by the support of PostgreSQL by Mailman, is there some news for a future release with this feature ?

Regards,

Revision history for this message
Barry Warsaw (barry) wrote :

On Nov 29, 2011, at 10:18 AM, Stephane Wirtel \(OpenERP\) wrote:

>I'm interested by the support of PostgreSQL by Mailman, is there some
>news for a future release with this feature ?

PostgreSQL support is in Mailman 3 bzr. It will be available in beta 1, but I
have no eta on that at the moment.

-Barry

Barry Warsaw (barry)
Changed in mailman:
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.