Merge lp:~wgrant/launchpad/sprb-new-model-columns into lp:launchpad/db-devel

[William Grant]

Drop SourcePackageRecipeBuildUpload, move its columns to SourcePackageRecipeBuild, and make storeUploadLog and storeBuildInfo function fully for SPRBs.

[William Grant (wgrant)]

This branch extends the prerequisite DB branch by adding the new dependencies, upload_log and pocket columns to the SourcePackageRecipeBuild model class. It also moves Build.storeUploadLog onto BuildBase, where it can be used by SPRB.

I had to alter BuildBase.storeBuildInfo to explicitly turn the dependencies str into a unicode, since plain Storm is less forgiving than its SQLObject compatibility layer. Since the method didn't appear to be tested anywhere, I've added some unit tests for the dependencies behaviour.

[Julian Edwards (julian-edwards)]

Looks good, thanks for this. Just a couple of comments:

1. s/exist/exists/ in the assertion text message.
2. You get brownie points if you can remove the commit from the tests - we found out that they slow the tests down massively so now I am on a jihad against them. Try using store.flush() or if that fails, try store.commit() as it's a bit closer to the metal than transaction.commit()

review: Approve

=== modified file '.bzrignore'
--- .bzrignore	2010-01-13 22:16:04 +0000
+++ .bzrignore	2010-02-15 21:43:48 +0000
@@ -58,3 +58,4 @@
 .subversion
 lib/canonical/buildd/launchpad-files
 .testrepository
+./pipes

=== modified file 'database/sampledata/current-dev.sql'
--- database/sampledata/current-dev.sql	2010-02-04 11:12:20 +0000
+++ database/sampledata/current-dev.sql	2010-02-15 21:43:48 +0000
@@ -9062,13 +9062,6 @@
 ALTER TABLE sourcepackagerecipebuildjob ENABLE TRIGGER ALL;
 
 
-ALTER TABLE sourcepackagerecipebuildupload DISABLE TRIGGER ALL;
-
-
-
-ALTER TABLE sourcepackagerecipebuildupload ENABLE TRIGGER ALL;
-
-
 ALTER TABLE sourcepackagerecipedata DISABLE TRIGGER ALL;
 
 

=== modified file 'database/sampledata/current.sql'
--- database/sampledata/current.sql	2010-02-04 11:12:20 +0000
+++ database/sampledata/current.sql	2010-02-15 21:43:48 +0000
@@ -8948,13 +8948,6 @@
 ALTER TABLE sourcepackagerecipebuildjob ENABLE TRIGGER ALL;
 
 
-ALTER TABLE sourcepackagerecipebuildupload DISABLE TRIGGER ALL;
-
-
-
-ALTER TABLE sourcepackagerecipebuildupload ENABLE TRIGGER ALL;
-
-
 ALTER TABLE sourcepackagerecipedata DISABLE TRIGGER ALL;
 
 

=== modified file 'database/schema/security.cfg'
--- database/schema/security.cfg	2010-02-15 13:33:06 +0000
+++ database/schema/security.cfg	2010-02-15 21:43:48 +0000
@@ -281,7 +281,6 @@
 public.sourcepackagerecipe              = SELECT, INSERT, UPDATE, DELETE
 public.sourcepackagerecipebuild         = SELECT, INSERT, UPDATE, DELETE
 public.sourcepackagerecipebuildjob      = SELECT, INSERT, UPDATE
-public.sourcepackagerecipebuildupload   = SELECT, INSERT, UPDATE, DELETE
 public.sourcepackagerecipedata          = SELECT, INSERT, UPDATE, DELETE
 public.sourcepackagerecipedatainstruction = SELECT, INSERT, UPDATE, DELETE
 public.specificationbranch              = SELECT, INSERT, UPDATE, DELETE
@@ -413,7 +412,6 @@
 public.sprint                           = SELECT
 public.sourcepackagereleasefile         = SELECT
 public.sourcepackagerecipebuild         = SELECT
-public.sourcepackagerecipebuildupload   = SELECT
 public.temporaryblobstorage             = SELECT, DELETE
 public.translationimportqueueentry      = SELECT
 

=== modified file 'lib/lp/buildmaster/model/buildbase.py'
--- lib/lp/buildmaster/model/buildbase.py	2010-02-10 15:02:42 +0000
+++ lib/lp/buildmaster/model/buildbase.py	2010-02-15 21:43:48 +0000
@@ -14,14 +14,18 @@
 import os
 import pytz
 import subprocess
+from cStringIO import StringIO
 
 from storm.store import Store
+from zope.component import getUtility
 from zope.security.proxy import removeSecurityProxy
 
 from canonical.config import config
 from canonical.database.constants import UTC_NOW
 from canonical.database.sqlbase import (
     clear_current_connection_cache, cursor, flush_database_updates)
+from canonical.launchpad.helpers import filenameToContentType
+from canonical.launchpad.interfaces.librarian import ILibraryFileAliasSet
 from canonical.librarian.utils import copy_and_close
 from lp.registry.interfaces.pocket import pocketsuffix
 from lp.soyuz.interfaces.build import BuildStatus
@@ -354,7 +358,31 @@
         # the time operations for duration.
         RIGHT_NOW = datetime.datetime.now(pytz.timezone('UTC'))
         self.buildduration = RIGHT_NOW - self.buildqueue_record.date_started
-        self.dependencies = slave_status.get('dependencies')
+        if slave_status.get('dependencies') is not None:
+            self.dependencies = unicode(slave_status.get('dependencies'))
+        else:
+            self.dependencies = None
+
+    def storeUploadLog(self, content):
+        """See `IBuildBase`."""
+        # The given content is stored in the librarian, restricted as
+        # necessary according to the targeted archive's privacy.  The content
+        # object's 'upload_log' attribute will point to the
+        # `LibrarianFileAlias`.
+
+        assert self.upload_log is None, (
+            "Upload log information already exists and cannot be overridden.")
+
+        filename = 'upload_%s_log.txt' % self.id
+        contentType = filenameToContentType(filename)
+        file_size = len(content)
+        file_content = StringIO(content)
+        restricted = self.is_private
+
+        library_file = getUtility(ILibraryFileAliasSet).create(
+            filename, file_size, file_content, contentType=contentType,
+            restricted=restricted)
+        self.upload_log = library_file
 
     def queueBuild(self, suspended=False):
         """See `IBuildBase`"""

=== modified file 'lib/lp/code/model/sourcepackagerecipebuild.py'
--- lib/lp/code/model/sourcepackagerecipebuild.py	2010-02-05 15:06:28 +0000
+++ lib/lp/code/model/sourcepackagerecipebuild.py	2010-02-15 21:43:48 +0000
@@ -12,10 +12,10 @@
 
 from canonical.database.constants import UTC_NOW
 from canonical.database.datetimecol import UtcDateTimeCol
-from canonical.database.enumcol import EnumCol
+from canonical.database.enumcol import DBEnum
 from canonical.launchpad.interfaces.lpstorm import IMasterStore
 
-from storm.locals import Int, Reference, Storm, TimeDelta
+from storm.locals import Int, Reference, Storm, TimeDelta, Unicode
 from storm.store import Store
 
 from zope.component import getUtility
@@ -62,8 +62,12 @@
     buildlog_id = Int(name='build_log', allow_none=True)
     buildlog = Reference(buildlog_id, 'LibraryFileAlias.id')
 
-    buildstate = EnumCol(
-        dbName='build_state', notNull=True, schema=BuildStatus)
+    buildstate = DBEnum(enum=BuildStatus, name='build_state')
+
+    dependencies = Unicode(allow_none=True)
+
+    upload_log_id = Int(name='upload_log', allow_none=True)
+    upload_log = Reference(upload_log_id, 'LibraryFileAlias.id')
 
     @property
     def current_component(self):
@@ -76,9 +80,6 @@
     distroseries_id = Int(name='distroseries', allow_none=True)
     distroseries = Reference(distroseries_id, 'DistroSeries.id')
 
-    # XXX wgrant 2010-01-15 bug=507751: Need a DB field for this.
-    dependencies = None
-
     sourcepackagename_id = Int(name='sourcepackagename', allow_none=True)
     sourcepackagename = Reference(
         sourcepackagename_id, 'SourcePackageName.id')
@@ -88,12 +89,7 @@
         """See `IBuildBase`."""
         return self.distroseries.distribution
 
-    @property
-    def pocket(self):
-        # XXX: JRV 2010-01-15 bug=507307: The database table really should
-        # have a pocket column, although this is not a big problem at the
-        # moment as recipe builds only happen for PPA's (so far).
-        return PackagePublishingPocket.RELEASE
+    pocket = DBEnum(enum=PackagePublishingPocket)
 
     recipe_id = Int(name='recipe', allow_none=False)
     recipe = Reference(recipe_id, 'SourcePackageRecipe.id')
@@ -112,13 +108,14 @@
         return results.one()
 
     def __init__(self, distroseries, sourcepackagename, recipe, requester,
-                 archive, date_created=None, date_first_dispatched=None,
+                 archive, pocket, date_created=None, date_first_dispatched=None,
                  date_built=None, builder=None,
                  build_state=BuildStatus.NEEDSBUILD, build_log=None,
                  build_duration=None):
         """Construct a SourcePackageRecipeBuild."""
         super(SourcePackageRecipeBuild, self).__init__()
         self.archive = archive
+        self.pocket = pocket
         self.buildduration = build_duration
         self.buildlog = build_log
         self.builder = builder
@@ -133,6 +130,7 @@
 
     @classmethod
     def new(cls, sourcepackage, recipe, requester, archive,
+            pocket=PackagePublishingPocket.RELEASE,
             date_created=None):
         """See `ISourcePackageRecipeBuildSource`."""
         store = IMasterStore(SourcePackageRecipeBuild)
@@ -144,6 +142,7 @@
             recipe,
             requester,
             archive,
+            pocket,
             date_created=date_created)
         store.add(spbuild)
         return spbuild
@@ -168,11 +167,6 @@
         # XXX: wgrant 2010-01-19 bug=507764: Need proper implementation.
         return datetime.timedelta(minutes=2)
 
-    def storeUploadLog(self, content):
-        """See `IBuildBase`."""
-        # XXX: wgrant 2010-01-20 bug=509892: Store in the DB.
-        return
-
     def notify(self, extra_info=None):
         """See `IBuildBase`."""
         # XXX: wgrant 2010-01-20 bug=509893: Implement this.

=== modified file 'lib/lp/soyuz/doc/build.txt'
--- lib/lp/soyuz/doc/build.txt	2010-02-15 12:59:55 +0000
+++ lib/lp/soyuz/doc/build.txt	2010-02-15 21:43:48 +0000
@@ -966,7 +966,7 @@
     >>> failedtoupload_build.storeUploadLog('something')
     Traceback (most recent call last):
     ...
-    AssertionError: Upload log information already exist and cannot be
+    AssertionError: Upload log information already exists and cannot be
     overridden.
 
 It's only possible to store another 'upload_log' content once the

=== modified file 'lib/lp/soyuz/model/build.py'
--- lib/lp/soyuz/model/build.py	2010-02-10 10:31:15 +0000
+++ lib/lp/soyuz/model/build.py	2010-02-15 21:43:48 +0000
@@ -7,7 +7,6 @@
 __all__ = ['Build', 'BuildSet']
 
 import apt_pkg
-from cStringIO import StringIO
 import datetime
 import logging
 import operator
@@ -32,10 +31,9 @@
 from canonical.launchpad.database.librarian import (
     LibraryFileAlias, LibraryFileContent)
 from canonical.launchpad.helpers import (
-     get_contact_email_addresses, filenameToContentType, get_email_template)
+     get_contact_email_addresses, get_email_template)
 from canonical.launchpad.interfaces.launchpad import (
     NotFoundError, ILaunchpadCelebrities)
-from canonical.launchpad.interfaces.librarian import ILibraryFileAliasSet
 from canonical.launchpad.mail import (
     simple_sendmail, format_address)
 from canonical.launchpad.webapp import canonical_url
@@ -668,27 +666,6 @@
                 fromaddress, toaddress, subject, message,
                 headers=extra_headers)
 
-    def storeUploadLog(self, content):
-        """See `IBuildBase`."""
-        # The given content is stored in the librarian, restricted as
-        # necessary according to the targeted archive's privacy.  The content
-        # object's 'upload_log' attribute will point to the
-        # `LibrarianFileAlias`.
-
-        assert self.upload_log is None, (
-            "Upload log information already exist and cannot be overridden.")
-
-        filename = 'upload_%s_log.txt' % self.id
-        contentType = filenameToContentType(filename)
-        file_size = len(content)
-        file_content = StringIO(content)
-        restricted = self.archive.private
-
-        library_file = getUtility(ILibraryFileAliasSet).create(
-            filename, file_size, file_content, contentType=contentType,
-            restricted=restricted)
-        self.upload_log = library_file
-
     def _getDebByFileName(self, filename):
         """Helper function to get a .deb LFA in the context of this build."""
         store = Store.of(self)

=== modified file 'lib/lp/soyuz/tests/test_build.py'
--- lib/lp/soyuz/tests/test_build.py	2010-02-03 16:21:57 +0000
+++ lib/lp/soyuz/tests/test_build.py	2010-02-15 21:43:48 +0000
@@ -8,8 +8,10 @@
 import unittest
 
 from storm.store import Store
+import transaction
 from zope.component import getUtility
 
+from canonical.database.constants import UTC_NOW
 from canonical.testing import LaunchpadZopelessLayer
 from lp.services.job.model.job import Job
 from lp.buildmaster.interfaces.buildbase import IBuildBase
@@ -21,6 +23,7 @@
 from lp.soyuz.model.buildpackagejob import BuildPackageJob
 from lp.soyuz.model.processor import ProcessorFamilySet
 from lp.soyuz.tests.test_publishing import SoyuzTestPublisher
+from lp.soyuz.tests.soyuzbuilddhelpers import WaitingSlave
 from lp.testing import TestCaseWithFactory
 
 
@@ -248,5 +251,46 @@
         self.assertContentEqual(builds, i386_builds)
 
 
+class TestStoreBuildInfo(TestCaseWithFactory):
+
+    layer = LaunchpadZopelessLayer
+
+    def setUp(self):
+        super(TestStoreBuildInfo, self).setUp()
+        self.publisher = SoyuzTestPublisher()
+        self.publisher.prepareBreezyAutotest()
+
+        gedit_src_hist = self.publisher.getPubSource(
+            sourcename="gedit", status=PackagePublishingStatus.PUBLISHED)
+        self.build = gedit_src_hist.createMissingBuilds()[0]
+
+        self.builder = self.factory.makeBuilder()
+        self.builder.setSlaveForTesting(WaitingSlave('BuildStatus.OK'))
+        self.build.buildqueue_record.builder = self.builder
+        self.build.buildqueue_record.setDateStarted(UTC_NOW)
+
+    def testDependencies(self):
+        """Verify that storeBuildInfo sets any dependencies."""
+        self.build.storeBuildInfo(None, {'dependencies': 'somepackage'})
+        self.assertIsNot(None, self.build.buildlog)
+        self.assertEqual(self.builder, self.build.builder)
+        self.assertEqual(u'somepackage', self.build.dependencies)
+        self.assertIsNot(None, self.build.datebuilt)
+        self.assertIsNot(None, self.build.buildduration)
+
+    def testWithoutDependencies(self):
+        """Verify that storeBuildInfo clears the build's dependencies."""
+        # Set something just to make sure that storeBuildInfo actually
+        # empties it.
+        self.build.dependencies = u'something'
+
+        self.build.storeBuildInfo(None, {})
+        self.assertIsNot(None, self.build.buildlog)
+        self.assertEqual(self.builder, self.build.builder)
+        self.assertIs(None, self.build.dependencies)
+        self.assertIsNot(None, self.build.datebuilt)
+        self.assertIsNot(None, self.build.buildduration)
+
+
 def test_suite():
     return unittest.TestLoader().loadTestsFromName(__name__)