← Back to team overview

launchpad-reviewers team mailing list archive

[Merge] lp:~wgrant/launchpad/antibfjo-8.5-mandatory-virtualized into lp:launchpad

 

William Grant has proposed merging lp:~wgrant/launchpad/antibfjo-8.5-mandatory-virtualized into lp:launchpad with lp:~wgrant/launchpad/antibfjo-8-kill as a prerequisite.

Commit message:
Remove model support for nullable BuildQueue.virtualized; that doesn't make sense.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~wgrant/launchpad/antibfjo-8.5-mandatory-virtualized/+merge/196639

Remove model support for nullable BuildQueue.virtualized; that doesn't make sense.
-- 
https://code.launchpad.net/~wgrant/launchpad/antibfjo-8.5-mandatory-virtualized/+merge/196639
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~wgrant/launchpad/antibfjo-8.5-mandatory-virtualized into lp:launchpad.
=== modified file 'lib/lp/buildmaster/doc/builder.txt'
--- lib/lp/buildmaster/doc/builder.txt	2013-11-14 07:36:26 +0000
+++ lib/lp/buildmaster/doc/builder.txt	2013-11-26 00:00:48 +0000
@@ -149,18 +149,3 @@
     >>> queue_sizes = builderset.getBuildQueueSizes()
     >>> print queue_sizes['virt']['386']
     (1L, datetime.timedelta(0, 600))
-
-Any BuildQueues with a null `virtualized` property are considered virtual
-by default.
-
-    >>> from zope.security.proxy import removeSecurityProxy
-    >>> recipe_bq = removeSecurityProxy(factory.makeSourcePackageRecipeBuild(
-    ...     distroseries=ubuntu.currentseries).queueBuild())
-    >>> recipe_bq.virtualized = None
-    >>> transaction.commit()
-    >>> queue_sizes = builderset.getBuildQueueSizes()
-
-The virtual queue size has increased accordingly:
-
-    >>> print queue_sizes['virt']['386']
-    (2L, datetime.timedelta(0, 1200))

=== modified file 'lib/lp/buildmaster/queuedepth.py'
--- lib/lp/buildmaster/queuedepth.py	2013-11-18 05:30:27 +0000
+++ lib/lp/buildmaster/queuedepth.py	2013-11-26 00:00:48 +0000
@@ -39,19 +39,10 @@
         builder_stats[(processor, virtualized)] = count
 
     builder_stats[(None, True)] = virtualized_total
-    # Jobs with a NULL virtualized flag should be treated the same as
-    # jobs where virtualized=TRUE.
-    builder_stats[(None, None)] = virtualized_total
     builder_stats[(None, False)] = builders_in_total - virtualized_total
     return builder_stats
 
 
-def normalize_virtualization(virtualized):
-    """Jobs with NULL virtualization settings should be treated the
-       same way as virtualized jobs."""
-    return virtualized is None or virtualized
-
-
 def get_free_builders_count(processor, virtualized):
     """How many builders capable of running jobs for the given processor
     and virtualization combination are idle/free at present?"""
@@ -62,7 +53,7 @@
             AND id NOT IN (
                 SELECT builder FROM BuildQueue WHERE builder IS NOT NULL)
             AND virtualized = %s
-        """ % sqlvalues(normalize_virtualization(virtualized))
+        """ % sqlvalues(virtualized)
     if processor is not None:
         query += """
             AND processor = %s
@@ -82,9 +73,7 @@
     :return: A (processor, virtualized) tuple which is the head job's
     platform or None if the JOI is the head job.
     """
-    my_platform = (
-        getattr(bq.processor, 'id', None),
-        normalize_virtualization(bq.virtualized))
+    my_platform = (getattr(bq.processor, 'id', None), bq.virtualized)
     query = """
         SELECT
             processor,
@@ -157,8 +146,7 @@
             AND BuildQueue.status = %s
             AND Builder.virtualized = %s
         """ % sqlvalues(
-            now, now, BuildQueueStatus.RUNNING,
-            normalize_virtualization(head_job_virtualized))
+            now, now, BuildQueueStatus.RUNNING, head_job_virtualized)
 
     if head_job_processor is not None:
         # Only look at builders with specific processor types.
@@ -174,7 +162,6 @@
 def get_pending_jobs_clauses(bq):
     """WHERE clauses for pending job queries, used for dipatch time
     estimation."""
-    virtualized = normalize_virtualization(bq.virtualized)
     clauses = """
         BuildQueue.status = %s
         AND (
@@ -183,15 +170,10 @@
             -- score is equal.
             BuildQueue.lastscore > %s OR
             (BuildQueue.lastscore = %s AND BuildQueue.id < %s))
-        -- The virtualized values either match or the job
-        -- does not care about virtualization and the job
-        -- of interest (JOI) is to be run on a virtual builder
-        -- (we want to prevent the execution of untrusted code
-        -- on native builders).
-        AND COALESCE(buildqueue.virtualized, TRUE) = %s
+        AND buildqueue.virtualized = %s
         """ % sqlvalues(
             BuildQueueStatus.WAITING, bq.lastscore, bq.lastscore, bq,
-            virtualized)
+            bq.virtualized)
     processor_clause = """
         AND (
             -- The processor values either match or the candidate
@@ -236,9 +218,7 @@
             # virtualization settings.
             return a == b
 
-    my_platform = (
-        getattr(bq.processor, 'id', None),
-        normalize_virtualization(bq.virtualized))
+    my_platform = (getattr(bq.processor, 'id', None), bq.virtualized)
     query = """
         SELECT
             BuildQueue.processor,
@@ -270,7 +250,6 @@
     #     duration by the total number of builders with the same
     #     virtualization setting because any one of them may run it.
     for processor, virtualized, job_count, delay in delays_by_platform:
-        virtualized = normalize_virtualization(virtualized)
         platform = (processor, virtualized)
         builder_count = builder_stats.get(platform, 0)
         if builder_count == 0:

=== modified file 'lib/lp/buildmaster/tests/test_queuedepth.py'
--- lib/lp/buildmaster/tests/test_queuedepth.py	2013-11-26 00:00:47 +0000
+++ lib/lp/buildmaster/tests/test_queuedepth.py	2013-11-26 00:00:48 +0000
@@ -19,7 +19,6 @@
     estimate_time_to_next_builder,
     get_builder_data,
     get_free_builders_count,
-    get_head_job_platform,
     )
 from lp.buildmaster.tests.test_buildqueue import find_job
 from lp.services.database.interfaces import IStore
@@ -861,68 +860,6 @@
         pg_platform = (postgres_job.processor.id, postgres_job.virtualized)
         self.assertEquals(pg_platform, postgres_job._getHeadJobPlatform())
 
-    def test_job_delay_for_unspecified_virtualization(self):
-        # Make sure that jobs with a NULL 'virtualized' flag get the same
-        # treatment as the ones with virtualized=TRUE.
-        # First toggle the 'virtualized' flag for all hppa jobs.
-        for build in self.builds:
-            bq = build.buildqueue_record
-            if bq.processor == self.hppa_proc:
-                removeSecurityProxy(bq).virtualized = True
-        job = self.makeCustomBuildQueue(
-            virtualized=True, estimated_duration=332,
-            sourcename=u'xxr-openssh-client', score=1050)
-        self.builds.append(job.specific_build)
-        # print_build_setup(self.builds)
-        #   ...
-        #   15,               flex, p: hppa, v: True e:0:13:00 *** s: 1039
-        #   16,               flex, p:  386, v:False e:0:14:00 *** s: 1042
-        #   17,           postgres, p: hppa, v: True e:0:15:00 *** s: 1045
-        #   18,           postgres, p:  386, v:False e:0:16:00 *** s: 1048
-        #   21, xxr-openssh-client, p: None, v: True e:0:05:32 *** s: 1050
-        #   20,      xx-recipe-zsh, p: None, v:False e:0:03:42 *** s: 1053
-
-        flex_build, flex_job = find_job(self, 'flex', 'hppa')
-        # The head job platform is the one of job #21 (xxr-openssh-client).
-        self.assertEquals(
-            (None, True), get_head_job_platform(removeSecurityProxy(flex_job)))
-        # The delay will be 900 (= 15*60) + 332 seconds
-        check_delay_for_job(self, flex_job, 1232)
-
-        # Now add a job with a NULL 'virtualized' flag. It should be treated
-        # like jobs with virtualized=TRUE.
-        job = self.makeCustomBuildQueue(
-            estimated_duration=111, sourcename=u'xxr-gwibber', score=1051,
-            virtualized=None)
-        self.builds.append(job.specific_build)
-        # print_build_setup(self.builds)
-        self.assertEqual(None, job.virtualized)
-        #   ...
-        #   15,               flex, p: hppa, v: True e:0:13:00 *** s: 1039
-        #   16,               flex, p:  386, v:False e:0:14:00 *** s: 1042
-        #   17,           postgres, p: hppa, v: True e:0:15:00 *** s: 1045
-        #   18,           postgres, p:  386, v:False e:0:16:00 *** s: 1048
-        #   21, xxr-openssh-client, p: None, v: True e:0:05:32 *** s: 1050
-        #   22,        xxr-gwibber, p: None, v: None e:0:01:51 *** s: 1051
-        #   20,      xx-recipe-zsh, p: None, v:False e:0:03:42 *** s: 1053
-
-        # The newly added 'xxr-gwibber' job is the new head job now.
-        self.assertEquals(
-            (None, None), get_head_job_platform(removeSecurityProxy(flex_job)))
-        # The newly added 'xxr-gwibber' job now weighs in as well and the
-        # delay is 900 (= 15*60) + (332+111)/2 seconds
-        check_delay_for_job(self, flex_job, 1121)
-
-        # The '386' flex job does not care about the 'xxr-gwibber' and
-        # 'xxr-openssh-client' jobs since the 'virtualized' values do not
-        # match.
-        flex_build, flex_job = find_job(self, 'flex', '386')
-        self.assertEquals(
-            (None, False),
-            get_head_job_platform(removeSecurityProxy(flex_job)))
-        # delay is 960 (= 16*60) + 222 seconds
-        check_delay_for_job(self, flex_job, 1182)
-
 
 class TestJobDispatchTimeEstimation(MultiArchBuildsBase):
     """Test estimated job delays with various processors."""
@@ -949,12 +886,12 @@
            16,               flex, p:  386, v: True e:0:14:00 *** s: 1042
            23,      xxr-apt-build, p: None, v: True e:0:12:56 *** s: 1043
            22,       xxr-cron-apt, p: None, v: True e:0:11:05 *** s: 1043
-           26,           xxr-cupt, p: None, v: None e:0:18:30 *** s: 1044
-           25,            xxr-apt, p: None, v: None e:0:16:38 *** s: 1044
-           24,       xxr-debdelta, p: None, v: None e:0:14:47 *** s: 1044
+           26,           xxr-cupt, p: None, v: True e:0:18:30 *** s: 1044
+           25,            xxr-apt, p: None, v: True e:0:16:38 *** s: 1044
+           24,       xxr-debdelta, p: None, v: True e:0:14:47 *** s: 1044
            17,           postgres, p: hppa, v:False e:0:15:00 *** s: 1045
            18,           postgres, p:  386, v: True e:0:16:00 *** s: 1048
-           21,         xxr-daptup, p: None, v: None e:0:09:14 *** s: 1051
+           21,         xxr-daptup, p: None, v: True e:0:09:14 *** s: 1051
            20,       xxr-auto-apt, p: None, v:False e:0:07:23 *** s: 1053
 
          p=processor, v=virtualized, e=estimated_duration, s=score
@@ -970,8 +907,7 @@
             sourcename=u'xxr-auto-apt', score=1053)
         self.builds.append(job.specific_build)
         job = self.makeCustomBuildQueue(
-            estimated_duration=554, sourcename=u'xxr-daptup', score=1051,
-            virtualized=None)
+            estimated_duration=554, sourcename=u'xxr-daptup', score=1051)
         self.builds.append(job.specific_build)
         job = self.makeCustomBuildQueue(
             estimated_duration=665, sourcename=u'xxr-cron-apt', score=1043)
@@ -980,16 +916,13 @@
             estimated_duration=776, sourcename=u'xxr-apt-build', score=1043)
         self.builds.append(job.specific_build)
         job = self.makeCustomBuildQueue(
-            estimated_duration=887, sourcename=u'xxr-debdelta', score=1044,
-            virtualized=None)
-        self.builds.append(job.specific_build)
-        job = self.makeCustomBuildQueue(
-            estimated_duration=998, sourcename=u'xxr-apt', score=1044,
-            virtualized=None)
-        self.builds.append(job.specific_build)
-        job = self.makeCustomBuildQueue(
-            estimated_duration=1110, sourcename=u'xxr-cupt', score=1044,
-            virtualized=None)
+            estimated_duration=887, sourcename=u'xxr-debdelta', score=1044)
+        self.builds.append(job.specific_build)
+        job = self.makeCustomBuildQueue(
+            estimated_duration=998, sourcename=u'xxr-apt', score=1044)
+        self.builds.append(job.specific_build)
+        job = self.makeCustomBuildQueue(
+            estimated_duration=1110, sourcename=u'xxr-cupt', score=1044)
         self.builds.append(job.specific_build)
 
         # Assign the same score to the '386' vim and apg build jobs.
@@ -1109,6 +1042,6 @@
         # Assign a job to it.
         assign_to_builder(self, 'gedit', 1, '386')
         # Dispatch the head job, making postgres/386 the new head job.
-        assign_to_builder(self, 'xxr-daptup', 1, None)
+        assign_to_builder(self, 'xxr-daptup', 2, None)
         postgres_build, postgres_job = find_job(self, 'postgres', '386')
         check_estimate(self, postgres_job, 120)

=== modified file 'lib/lp/code/model/branch.py'
--- lib/lp/code/model/branch.py	2013-11-15 06:31:01 +0000
+++ lib/lp/code/model/branch.py	2013-11-26 00:00:48 +0000
@@ -1263,29 +1263,22 @@
             TranslationTemplatesBuild,
             )
 
+        # Remove BranchJobs.
         store = Store.of(self)
         affected_jobs = Select(
             [BranchJob.jobID],
             And(BranchJob.job == Job.id, BranchJob.branch == self))
+        store.find(Job, Job.id.is_in(affected_jobs)).remove()
 
-        # Find BuildFarmJobs to delete.
+        # Delete TranslationTemplatesBuilds, their BuildFarmJobs and
+        # their BuildQueues.
         bfjs = store.find(
             (BuildFarmJob.id,),
             TranslationTemplatesBuild.build_farm_job_id == BuildFarmJob.id,
             TranslationTemplatesBuild.branch == self)
         bfj_ids = [bfj[0] for bfj in bfjs]
-
-        # Delete BuildQueue entries for affected Jobs.  They would pin
-        # the affected Jobs in the database otherwise.
         store.find(
-            BuildQueue,
-            Or(
-                BuildQueue.jobID.is_in(affected_jobs),
-                BuildQueue._build_farm_job_id.is_in(bfj_ids))).remove()
-
-        # Delete Jobs.  Their BranchJobs cascade along in the database.
-        store.find(Job, Job.id.is_in(affected_jobs)).remove()
-
+            BuildQueue, BuildQueue._build_farm_job_id.is_in(bfj_ids)).remove()
         store.find(
             TranslationTemplatesBuild,
             TranslationTemplatesBuild.branch == self).remove()

=== modified file 'lib/lp/soyuz/stories/ppa/xx-copy-packages.txt'
--- lib/lp/soyuz/stories/ppa/xx-copy-packages.txt	2013-09-30 02:59:29 +0000
+++ lib/lp/soyuz/stories/ppa/xx-copy-packages.txt	2013-11-26 00:00:48 +0000
@@ -876,10 +876,10 @@
     >>> run_copy_jobs()
     DEBUG Packages copied to PPA for James Blackwell Sandbox:
     DEBUG pmount 0.1-1 in warty
+    INFO ... raised CannotCopy: pmount 0.1-1 in hoary
+    (same version already building in the destination archive for Warty)
     INFO ... raised CannotCopy: pmount 0.1-1 in grumpy
     (same version already building in the destination archive for Warty)
-    INFO ... raised CannotCopy: pmount 0.1-1 in hoary
-    (same version already building in the destination archive for Warty)
 
 Due to the copy error, nothing was copied to the destination PPA, not
 even the 'warty' source, which was not denied.


Follow ups