Merge lp:~abompard/mailman/sqlalchemy into lp:~raj-abhilash1/mailman/sqlalchemy

[Aurélien Bompard]

Here are my changes for automatic schema migration. The "migrate" subcommand is now useless, and I've fixed the "alembic revision" command, you can now create a new DB version with:
  alembic -c src/mailman/config/schema.cfg revision --autogenerate

It currently does generate something, by the way (it shouldn't if everything was perfectly ported from Storm to SQLAlchemy). I need to look into that (but first, unit tests).

[lp:~abompard/mailman/sqlalchemy]

7276. By Abhilash Raj on 2014-10-02

Merge alembic setup from abompard

7277. By Abhilash Raj on 2014-10-02

Merge barry\'s branch with test fixes and clean code

7278. By Abhilash Raj on 2014-10-03

add central alembic config

=== modified file 'MANIFEST.in'
--- MANIFEST.in	2014-10-02 12:50:42 +0000
+++ MANIFEST.in	2014-10-08 08:59:34 +0000
@@ -13,5 +13,5 @@
 prune parts
 include MANIFEST.in
 include src/mailman/testing/config.pck
-include src/mailman/databases/alembic/scripts.py.mako
-include src/mailman/databases/alembic/versions/*.py
+include src/mailman/database/alembic/script.py.mako
+include src/mailman/database/alembic/versions/*.py

=== modified file 'src/mailman/commands/docs/conf.rst'
--- src/mailman/commands/docs/conf.rst	2014-09-28 00:17:05 +0000
+++ src/mailman/commands/docs/conf.rst	2014-10-08 08:59:34 +0000
@@ -22,7 +22,7 @@
 command without any options.
 
     >>> command.process(FakeArgs)
-    [logging.archiver] path: mailman.log
+    [logging.dbmigration] path: mailman.log
     ...
     [passwords] password_length: 8
     ...
@@ -43,12 +43,14 @@
     >>> FakeArgs.section = None
     >>> FakeArgs.key = 'path'
     >>> command.process(FakeArgs)
+    [logging.dbmigration] path: mailman.log
     [logging.archiver] path: mailman.log
     [logging.locks] path: mailman.log
     [logging.mischief] path: mailman.log
     [logging.config] path: mailman.log
     [logging.error] path: mailman.log
     [logging.smtp] path: smtp.log
+    [logging.database] path: mailman.log
     [logging.http] path: mailman.log
     [logging.root] path: mailman.log
     [logging.fromusenet] path: mailman.log

=== modified file 'src/mailman/config/alembic.cfg'
--- src/mailman/config/alembic.cfg	2014-10-03 16:26:50 +0000
+++ src/mailman/config/alembic.cfg	2014-10-08 08:59:34 +0000
@@ -19,39 +19,3 @@
 # a source .py file to be detected as revisions in the
 # versions/ directory
 # sourceless = false
-
-
-# Logging configuration
-[loggers]
-keys = root,sqlalchemy,alembic
-
-[handlers]
-keys = console
-
-[formatters]
-keys = generic
-
-[logger_root]
-level = WARN
-handlers = console
-qualname =
-
-[logger_sqlalchemy]
-level = WARN
-handlers = console
-qualname = sqlalchemy.engine
-
-[logger_alembic]
-level = WARN
-handlers = console
-qualname = alembic
-
-[handler_console]
-class = StreamHandler
-args = (sys.stderr,)
-level = WARN
-formatter = generic
-
-[formatter_generic]
-format = %(levelname)-5.5s [%(name)s] %(message)s
-datefmt = %H:%M:%S

=== modified file 'src/mailman/config/schema.cfg'
--- src/mailman/config/schema.cfg	2014-10-02 04:15:36 +0000
+++ src/mailman/config/schema.cfg	2014-10-08 08:59:34 +0000
@@ -204,10 +204,6 @@
 url: sqlite:///$DATA_DIR/mailman.db
 debug: no
 
-# Where can we find the Alembic migration scripts?  The `python:` schema means
-# that this is a module path relative to the Mailman 3 source installation.
-alembic_scripts: mailman.database:alembic
-
 
 [logging.template]
 # This defines various log settings.  The options available are:
@@ -242,6 +238,8 @@
 # - smtp-failure    --  Unsuccessful SMTP activity
 # - subscribe       --  Information about leaves/joins
 # - vette           --  Message vetting information
+# - database        --  Database activity
+# - dbmigration     --  Database migrations
 format: %(asctime)s (%(process)d) %(message)s
 datefmt: %b %d %H:%M:%S %Y
 propagate: no
@@ -306,6 +304,12 @@
 
 [logging.vette]
 
+[logging.database]
+level: warn
+
+[logging.dbmigration]
+level: warn
+
 
 [webservice]
 # The hostname at which admin web service resources are exposed.

=== modified file 'src/mailman/core/logging.py'
--- src/mailman/core/logging.py	2014-04-28 15:23:35 +0000
+++ src/mailman/core/logging.py	2014-10-08 08:59:34 +0000
@@ -126,6 +126,10 @@
             continue
         if sub_name == 'locks':
             log = logging.getLogger('flufl.lock')
+        elif sub_name == 'database':
+            log = logging.getLogger('sqlalchemy')
+        elif sub_name == 'dbmigration':
+            log = logging.getLogger('alembic')
         else:
             logger_name = 'mailman.' + sub_name
             log = logging.getLogger(logger_name)

=== modified file 'src/mailman/database/alembic/env.py'
--- src/mailman/database/alembic/env.py	2014-10-03 16:26:50 +0000
+++ src/mailman/database/alembic/env.py	2014-10-08 08:59:34 +0000
@@ -28,7 +28,6 @@
 
 from alembic import context
 from contextlib import closing
-from logging.config import fileConfig
 from sqlalchemy import create_engine
 
 from mailman.core import initialize
@@ -39,8 +38,6 @@
 from mailman.utilities.string import expand
 
 
-fileConfig(alembic_cfg.config_file_name)
-
 

 def run_migrations_offline():
     """Run migrations in 'offline' mode.

=== modified file 'src/mailman/database/base.py'
--- src/mailman/database/base.py	2014-10-03 16:26:50 +0000
+++ src/mailman/database/base.py	2014-10-08 08:59:34 +0000
@@ -31,7 +31,6 @@
 from zope.interface import implementer
 
 from mailman.config import config
-from mailman.database.alembic import alembic_cfg
 from mailman.interfaces.database import IDatabase
 from mailman.utilities.string import expand
 
@@ -91,15 +90,6 @@
         """
         pass
 
-    def stamp(self, debug=False):
-        """Stamp the database with the latest Alembic version."""
-        # Newly created databases don't need migrations from Alembic, since
-        # create_all() ceates the latest schema.  This patches the database
-        # with the latest Alembic version to add an entry in the
-        # alembic_version table.
-        command.stamp(alembic_cfg, 'head')
-
-
     def initialize(self, debug=None):
         """See `IDatabase`."""
         # Calculate the engine url.

=== modified file 'src/mailman/database/factory.py'
--- src/mailman/database/factory.py	2014-10-03 16:26:50 +0000
+++ src/mailman/database/factory.py	2014-10-08 08:59:34 +0000
@@ -70,8 +70,7 @@
 
     def __init__(self, database):
         self.database = database
-        self.alembic_cfg = alembic_cfg
-        self.script = ScriptDirectory.from_config(self.alembic_cfg)
+        self.script = ScriptDirectory.from_config(alembic_cfg)
 
     def get_storm_schema_version(self):
         md = MetaData()
@@ -82,8 +81,18 @@
         last_version = self.database.store.query(Version.c.version).filter(
                 Version.c.component == "schema"
                 ).order_by(Version.c.version.desc()).first()
+        # Don't leave open transactions or they will block any schema change
+        self.database.commit()
         return last_version
 
+    def _create(self):
+        # initial DB creation
+        Model.metadata.create_all(self.database.engine)
+        command.stamp(alembic_cfg, "head")
+
+    def _upgrade(self):
+        command.upgrade(alembic_cfg, "head")
+
     def setup_db(self):
         context = MigrationContext.configure(self.database.store.connection())
         current_rev = context.get_current_revision()
@@ -95,8 +104,7 @@
             storm_version = self.get_storm_schema_version()
             if storm_version is None:
                 # initial DB creation
-                Model.metadata.create_all(self.database.engine)
-                command.stamp(self.alembic_cfg, "head")
+                self._create()
             else:
                 # DB from a previous version managed by Storm
                 if storm_version.version < self.LAST_STORM_SCHEMA_VERSION:
@@ -106,9 +114,9 @@
                             "Mailman beta release")
                 # Run migrations to remove the Storm-specific table and
                 # upgrade to SQLAlchemy & Alembic
-                command.upgrade(self.alembic_cfg, "head")
+                self._upgrade()
         elif current_rev != head_rev:
-            command.upgrade(self.alembic_cfg, "head")
+            self._upgrade()
         return head_rev
 
 

=== added directory 'src/mailman/database/tests'
=== added file 'src/mailman/database/tests/__init__.py'
=== added file 'src/mailman/database/tests/test_factory.py'
--- src/mailman/database/tests/test_factory.py	1970-01-01 00:00:00 +0000
+++ src/mailman/database/tests/test_factory.py	2014-10-08 08:59:34 +0000
@@ -0,0 +1,134 @@
+# Copyright (C) 2013-2014 by the Free Software Foundation, Inc.
+#
+# This file is part of GNU Mailman.
+#
+# GNU Mailman is free software: you can redistribute it and/or modify it under
+# the terms of the GNU General Public License as published by the Free
+# Software Foundation, either version 3 of the License, or (at your option)
+# any later version.
+#
+# GNU Mailman is distributed in the hope that it will be useful, but WITHOUT
+# ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+# FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+# more details.
+#
+# You should have received a copy of the GNU General Public License along with
+# GNU Mailman.  If not, see <http://www.gnu.org/licenses/>.
+
+"""Test database schema migrations"""
+
+from __future__ import absolute_import, print_function, unicode_literals
+
+__metaclass__ = type
+__all__ = [
+    ]
+
+
+import unittest
+import types
+
+import alembic.command
+from mock import Mock
+from sqlalchemy import MetaData, Table, Column, Integer, Unicode
+
+from mailman.config import config
+from mailman.testing.layers import ConfigLayer
+from mailman.database.factory import SchemaManager, _reset
+from mailman.database.sqlite import SQLiteDatabase
+from mailman.database.alembic import alembic_cfg
+from mailman.database.model import Model
+
+
+

+class TestSchemaManager(unittest.TestCase):
+
+    layer = ConfigLayer
+
+    def setUp(self):
+        # Drop the existing database
+        Model.metadata.drop_all(config.db.engine)
+        md = MetaData()
+        md.reflect(bind=config.db.engine)
+        if "alembic_version" in md.tables:
+            md.tables["alembic_version"].drop(config.db.engine)
+        self.schema_mgr = SchemaManager(config.db)
+
+    def tearDown(self):
+        if "version" in Model.metadata.tables:
+            version = Model.metadata.tables["version"]
+            version.drop(config.db.engine, checkfirst=True)
+            Model.metadata.remove(version)
+        # Restore a virgin DB
+        Model.metadata.create_all(config.db.engine)
+
+
+    def _table_exists(self, tablename):
+        md = MetaData()
+        md.reflect(bind=config.db.engine)
+        return tablename in md.tables
+
+    def _create_storm_version_table(self, revision):
+        version_table = Table("version", Model.metadata,
+                Column("id", Integer, primary_key=True),
+                Column("component", Unicode),
+                Column("version", Unicode),
+                )
+        version_table.create(config.db.engine)
+        config.db.store.execute(version_table.insert().values(
+                component='schema', version=revision))
+        config.db.commit()
+
+
+    def test_current_db(self):
+        """The database is already at the latest version"""
+        alembic.command.stamp(alembic_cfg, "head")
+        self.schema_mgr._create = Mock()
+        self.schema_mgr._upgrade = Mock()
+        self.schema_mgr.setup_db()
+        self.assertFalse(self.schema_mgr._create.called)
+        self.assertFalse(self.schema_mgr._upgrade.called)
+
+    def test_initial(self):
+        """No existing database"""
+        self.assertFalse(self._table_exists("mailinglist"))
+        self.assertFalse(self._table_exists("alembic_version"))
+        self.schema_mgr._upgrade = Mock()
+        self.schema_mgr.setup_db()
+        self.assertFalse(self.schema_mgr._upgrade.called)
+        self.assertTrue(self._table_exists("mailinglist"))
+        self.assertTrue(self._table_exists("alembic_version"))
+
+    def test_storm(self):
+        """Existing Storm database"""
+        Model.metadata.create_all(config.db.engine)
+        self._create_storm_version_table(
+                self.schema_mgr.LAST_STORM_SCHEMA_VERSION)
+        self.schema_mgr._create = Mock()
+        self.schema_mgr.setup_db()
+        self.assertFalse(self.schema_mgr._create.called)
+        self.assertTrue(self._table_exists("mailinglist")
+                    and self._table_exists("alembic_version")
+                    and not self._table_exists("version"))
+
+    def test_old_storm(self):
+        """Existing Storm database in an old version"""
+        Model.metadata.create_all(config.db.engine)
+        self._create_storm_version_table("001")
+        self.schema_mgr._create = Mock()
+        self.assertRaises(RuntimeError, self.schema_mgr.setup_db)
+        self.assertFalse(self.schema_mgr._create.called)
+
+    def test_old_db(self):
+        """The database is in an old revision, must upgrade"""
+        alembic.command.stamp(alembic_cfg, "head")
+        md = MetaData()
+        md.reflect(bind=config.db.engine)
+        config.db.store.execute(md.tables["alembic_version"].delete())
+        config.db.store.execute(md.tables["alembic_version"].insert().values(
+                version_num="dummyrevision"))
+        config.db.commit()
+        self.schema_mgr._create = Mock()
+        self.schema_mgr._upgrade = Mock()
+        self.schema_mgr.setup_db()
+        self.assertFalse(self.schema_mgr._create.called)
+        self.assertTrue(self.schema_mgr._upgrade.called)

=== modified file 'src/mailman/testing/layers.py'
--- src/mailman/testing/layers.py	2014-09-27 22:16:22 +0000
+++ src/mailman/testing/layers.py	2014-10-08 08:59:34 +0000
@@ -47,13 +47,16 @@
 
 from lazr.config import as_boolean
 from pkg_resources import resource_string
+from sqlalchemy import MetaData
 from textwrap import dedent
+from zope import event
 from zope.component import getUtility
 
 from mailman.config import config
 from mailman.core import initialize
 from mailman.core.initialize import INHIBIT_CONFIG_FILE
 from mailman.core.logging import get_handler
+from mailman.database.model import Model
 from mailman.database.transaction import transaction
 from mailman.interfaces.domain import IDomainManager
 from mailman.testing.helpers import (
@@ -98,7 +101,9 @@
         # Set up the basic configuration stuff.  Turn off path creation until
         # we've pushed the testing config.
         config.create_paths = False
-        initialize.initialize_1(INHIBIT_CONFIG_FILE)
+        if not event.subscribers:
+            # only if not yet initialized by another layer
+            initialize.initialize_1(INHIBIT_CONFIG_FILE)
         assert cls.var_dir is None, 'Layer already set up'
         # Calculate a temporary VAR_DIR directory so that run-time artifacts
         # of the tests won't tread on the installation's data.  This also

=== modified file 'src/mailman/testing/testing.cfg'
--- src/mailman/testing/testing.cfg	2014-10-03 16:26:50 +0000
+++ src/mailman/testing/testing.cfg	2014-10-08 08:59:34 +0000
@@ -18,9 +18,9 @@
 # A testing configuration.
 
 # For testing against PostgreSQL.
-# [database]
-# class: mailman.database.postgresql.PostgreSQLDatabase
-# url: postgres://maxking:maxking@localhost/mailman_test
+[database]
+class: mailman.database.postgresql.PostgreSQLDatabase
+url: postgresql://maxking:maxking@localhost/mailman_test
 
 [mailman]
 site_owner: [e-mail]