launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #04522
[Merge] lp:~stub/launchpad/trivial into lp:launchpad
Stuart Bishop has proposed merging lp:~stub/launchpad/trivial into lp:launchpad.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
Related bugs:
Bug #569217 in Launchpad itself: "Remove os.walk copy when we switch to Python 2.6"
https://bugs.launchpad.net/launchpad/+bug/569217
Bug #809636 in Launchpad itself: "no method for having optional db patches"
https://bugs.launchpad.net/launchpad/+bug/809636
Bug #815706 in Launchpad itself: "Update applied DB patch checks for new deployment process"
https://bugs.launchpad.net/launchpad/+bug/815706
Bug #820291 in Launchpad itself: "BugHeatUpdater attempts to update all outdated bugs at once"
https://bugs.launchpad.net/launchpad/+bug/820291
For more details, see:
https://code.launchpad.net/~stub/launchpad/trivial/+merge/70840
= Summary =
On startup, Launchpad checks LaunchpadDatabaseRevision to see if the deployed schema is compatible with the schema.
These checks are not compatible with the fastdowntime deployment process, as code and database schema updates do not happen in sync.
== Proposed fix ==
Turn off the checks entirely. We might be able to work out some rules, but that don't gain us enough to justify the inevitable confusion and may be a footgun.
== Pre-implementation notes ==
== Implementation details ==
== Tests ==
== Demo and Q/A ==
= Launchpad lint =
Checking for conflicts and issues in changed files.
Linting changed files:
lib/canonical/launchpad/webapp/configure.zcml
lib/canonical/testing/layers.py
--
https://code.launchpad.net/~stub/launchpad/trivial/+merge/70840
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~stub/launchpad/trivial into lp:launchpad.
=== removed file 'lib/canonical/database/ftests/test_revision.py'
--- lib/canonical/database/ftests/test_revision.py 2010-10-04 19:50:45 +0000
+++ lib/canonical/database/ftests/test_revision.py 1970-01-01 00:00:00 +0000
@@ -1,128 +0,0 @@
-# Copyright 2009 Canonical Ltd. This software is licensed under the
-# GNU Affero General Public License version 3 (see the file LICENSE).
-
-"""Tests for the revision module."""
-
-__metaclass__ = type
-__all__ = []
-
-from glob import glob
-import os.path
-import re
-import unittest
-
-import psycopg2
-
-from canonical.config import config
-from canonical.database.sqlbase import cursor
-from canonical.database.revision import (
- confirm_dbrevision, InvalidDatabaseRevision,
- )
-from canonical.testing.layers import LaunchpadZopelessLayer
-
-class TestRevision(unittest.TestCase):
- layer = LaunchpadZopelessLayer
-
- def setUp(self):
- schema_dir = os.path.join(config.root, 'database', 'schema')
- baseline, = glob(os.path.join(schema_dir, 'launchpad-*-00-0.sql'))
- match = re.search('launchpad-(\d+)-00-0.sql', baseline)
- self.major = int(match.group(1))
- self.cur = cursor()
-
- def test_confirm_dbrevision(self):
- # Function should not raise an exception with a fresh Launchpad
- # database. This test will fail if the test database is old and
- # needs to be rebuilt.
- confirm_dbrevision(self.cur)
-
- def test_confirm_dbrevision2(self):
- # Create a fake database patch on the file system and confirm
- # an exception is raised
- path = os.path.join(
- config.root, 'database', 'schema',
- 'patch-%04d-96-0.sql' % self.major
- )
- self.failIf(
- os.path.exists(path),
- '%s already exists but it is reserved for this test' % path
- )
- fake_patch = open(path, 'w')
- try:
- print >> fake_patch, """
- Delete this file - it is garbage from a test that died before
- it could clean up properly.
- """
- fake_patch.close()
- self.failUnlessRaises(
- InvalidDatabaseRevision, confirm_dbrevision, self.cur
- )
- finally:
- os.remove(path)
-
- def test_confirm_dbrevision3(self):
- # Create a record of a fake database patch that does not exist on the
- # filesystem and onfirm an exception is raised
- self.cur.execute(
- "INSERT INTO LaunchpadDatabaseRevision VALUES (%s, 96, 0)",
- (self.major,)
- )
- self.failUnlessRaises(
- InvalidDatabaseRevision, confirm_dbrevision, self.cur
- )
-
- def test_confirm_dbrevision4(self):
- # Create a record of a fake database patch of the sort that is
- # applied to the live systems (non zero 'patch' number). It
- # should not raise an exeption in this case.
- self.cur.execute(
- "INSERT INTO LaunchpadDatabaseRevision VALUES (%s, 96, 1)",
- (self.major,)
- )
- confirm_dbrevision(self.cur)
-
- def test_confirm_dbrevision5(self):
- # Records of earlier 'major' patches stored in the database are
- # ignored.
- self.cur.execute(
- "INSERT INTO LaunchpadDatabaseRevision VALUES (%s, 96, 0)",
- (self.major-1,)
- )
- confirm_dbrevision(self.cur)
-
- def test_confirm_dbrevision6(self):
- # Records of later 'major' patches stored in the database are
- # not ignored.
- self.cur.execute(
- "INSERT INTO LaunchpadDatabaseRevision VALUES (%s, 96, 0)",
- (self.major+1,)
- )
- self.failUnlessRaises(
- InvalidDatabaseRevision, confirm_dbrevision, self.cur
- )
-
- def test_assert_patch_applied(self):
- # assert_patch_applied() is a stored procedure that raises
- # an exception if the given patch has not been applied to the
- # database. It is used in slonik scripts to catch and abort
- # on a bad patch.
- self.cur.execute("""
- INSERT INTO LaunchpadDatabaseRevision VALUES (999,999,999)
- """)
-
- # First statement doesn't raise an exception, as according to
- # the LaunchpadDatabaseRevision table the patch has been applied.
- self.cur.execute("SELECT assert_patch_applied(999,999,999)")
-
- # This second statement raises an exception, as no such patch
- # has been applied.
- self.failUnlessRaises(
- psycopg2.Error, self.cur.execute,
- "SELECT assert_patch_applied(1,999,999)")
-
-
-def test_suite():
- suite = unittest.TestSuite()
- suite.addTest(unittest.makeSuite(TestRevision))
- return suite
-
=== removed file 'lib/canonical/database/revision.py'
--- lib/canonical/database/revision.py 2009-12-08 06:25:40 +0000
+++ lib/canonical/database/revision.py 1970-01-01 00:00:00 +0000
@@ -1,88 +0,0 @@
-# Copyright 2009 Canonical Ltd. This software is licensed under the
-# GNU Affero General Public License version 3 (see the file LICENSE).
-
-"""Code dealing with the Launchpad database patch level."""
-
-__metaclass__ = type
-__all__ = ['confirm_dbrevision', 'InvalidDatabaseRevision']
-
-from glob import glob
-import os.path
-import re
-
-from canonical.config import config
-from canonical.database.sqlbase import connect
-
-
-class InvalidDatabaseRevision(Exception):
- """Exception raised by confirm_dbrevision."""
-
-
-def confirm_dbrevision(cur):
- """Check that the database we are connected to is the same
- database patch level as expected by the code.
-
- Raises an InvalidDatabaseRevision exception if the database patch level
- is not what is expected.
- """
- # Get a list of patches the code expects to have been applied from the
- # filesystem.
- schema_dir = os.path.join(config.root, 'database', 'schema')
- patches_glob = os.path.join(schema_dir, 'patch-*-*-*.sql')
- fs_patches = []
- for patch_file in glob(patches_glob):
- match = re.search('patch-(\d+)-(\d+)-(\d+).sql', patch_file)
- if match is None:
- raise InvalidDatabaseRevision("Bad patch name %r" % (patch_file,))
- fs_patches.append(
- (int(match.group(1)), int(match.group(2)), int(match.group(3)))
- )
- fs_patches.sort()
-
- # Get a list of patches that have been applied to the database.
- # We skip any patches from earlier 'major' revision levels, as they
- # are no longer stored on the filesystem.
- fs_major = fs_patches[0][0]
- cur.execute("""
- SELECT major, minor, patch FROM LaunchpadDatabaseRevision
- ORDER BY major, minor, patch
- """)
- db_patches = [
- (major, minor, patch) for major, minor, patch in cur.fetchall()
- if major >= fs_major
- ]
-
- # Raise an exception if we have a patch on the filesystem that has not
- # been applied to the database.
- for patch_tuple in fs_patches:
- if patch_tuple not in db_patches:
- raise InvalidDatabaseRevision(
- "patch-%04d-%02d-%d.sql has not been applied to the "
- "database. You probably want to run 'make schema'."
- % patch_tuple)
-
- # Raise an exeption if we have a patch applied to the database that
- # cannot be found on the filesystem. We ignore patches with a non-zero
- # 'patch' part to its version number as these patches are used to apply
- # fixes to the live database that should be backported to the code trunk.
- # (This may be problematic with some systems such as the authserver and
- # librarian that we roll out infrequently. We may want a less strict
- # version of this function to check database revisions with these
- # services).
- for patch_tuple in db_patches:
- if patch_tuple[2] == 0 and patch_tuple not in fs_patches:
- raise InvalidDatabaseRevision(
- "patch-%04d-%02d-%d.sql has been applied to the database "
- "but does not exist in this source code tree. You probably "
- "want to run 'make schema'." % patch_tuple)
-
-
-def confirm_dbrevision_on_startup(*ignored):
- """Event handler that calls confirm_dbrevision"""
- con = connect(config.launchpad.dbuser)
- try:
- cur = con.cursor()
- confirm_dbrevision(cur)
- finally:
- con.close()
-
=== modified file 'lib/canonical/launchpad/webapp/configure.zcml'
--- lib/canonical/launchpad/webapp/configure.zcml 2011-07-01 15:36:23 +0000
+++ lib/canonical/launchpad/webapp/configure.zcml 2011-08-09 09:57:24 +0000
@@ -351,13 +351,6 @@
handler="canonical.launchpad.webapp.sigdumpmem.setup_sigdumpmem"
/>
-
- <!-- Confirm the database is the correct revision level -->
- <subscriber
- for="zope.app.appsetup.IProcessStartingEvent"
- handler="canonical.database.revision.confirm_dbrevision_on_startup"
- />
-
<!-- Confirm that no main thread event handlers use the connection cache -->
<subscriber
for="zope.app.appsetup.IProcessStartingEvent"
=== modified file 'lib/canonical/testing/layers.py'
--- lib/canonical/testing/layers.py 2011-08-05 01:15:21 +0000
+++ lib/canonical/testing/layers.py 2011-08-09 09:57:24 +0000
@@ -106,12 +106,7 @@
ConfigFixture,
ConfigUseFixture,
)
-from canonical.database.revision import (
- confirm_dbrevision,
- confirm_dbrevision_on_startup,
- )
from canonical.database.sqlbase import (
- cursor,
session_store,
ZopelessTransactionManager,
)
@@ -226,9 +221,6 @@
main_store = getUtility(IStoreSelector).get(MAIN_STORE, DEFAULT_FLAVOR)
assert main_store is not None, 'Failed to reconnect'
- # Confirm the database has the right patchlevel
- confirm_dbrevision(cursor())
-
# Confirm that SQLOS is again talking to the database (it connects
# as soon as SQLBase._connection is accessed
r = main_store.execute('SELECT count(*) FROM LaunchpadDatabaseRevision')
@@ -1884,10 +1876,6 @@
@classmethod
def _runAppServer(cls):
"""Start the app server using runlaunchpad.py"""
- # The app server will not start at all if the database hasn't been
- # correctly patched. The app server will make exactly this check,
- # doing it here makes the error more obvious.
- confirm_dbrevision_on_startup()
_config = cls.appserver_config
cmd = [
os.path.join(_config.root, 'bin', 'run'),
=== removed file 'utilities/check-db-revision.py'
--- utilities/check-db-revision.py 2010-04-27 19:48:39 +0000
+++ utilities/check-db-revision.py 1970-01-01 00:00:00 +0000
@@ -1,22 +0,0 @@
-#!/usr/bin/python -S
-# Copyright 2010 Canonical Ltd. All rights reserved.
-
-"""
-Check that the database revision of the current branch matches the current
-database schema number.
-"""
-
-import _pythonpath
-import sys
-
-from canonical.database.revision import (
- confirm_dbrevision_on_startup, InvalidDatabaseRevision)
-
-try:
- confirm_dbrevision_on_startup()
-except InvalidDatabaseRevision, err:
- print "Oops, we are trying to use an invalid database revision!"
- print err
- sys.exit(1)
-else:
- sys.exit(0)
