launchpad-reviewers team mailing list archive

[William Grant <me@xxxxxxxxxxxxxxxxxx>]

William Grant has proposed merging lp:~wgrant/launchpad/no-lucilleconfig-stayofexecution into lp:launchpad/devel with lp:~wgrant/launchpad/replace-lucilleconfig-directories as a prerequisite.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)


Another stage of my anti-lucilleconfig tirade, this branch obsoletes Distribution.lucilleconfig's pendingremovalduration key.

As suggested in https://bugs.launchpad.net/soyuz/+bug/45270/comments/6, the stay of execution is now hardcoded to one day. Dominator no longer has any use for its config, so it has been unthreaded.
-- 
https://code.launchpad.net/~wgrant/launchpad/no-lucilleconfig-stayofexecution/+merge/38653
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~wgrant/launchpad/no-lucilleconfig-stayofexecution into lp:launchpad/devel.

=== modified file 'lib/lp/archivepublisher/config.py'
--- lib/lp/archivepublisher/config.py	2010-10-17 12:56:03 +0000
+++ lib/lp/archivepublisher/config.py	2010-10-17 12:56:07 +0000
@@ -120,8 +120,6 @@
         self._distroconfig.readfp(strio)
         strio.close()
 
-        self._extractConfigInfo()
-
     def distroSeriesNames(self):
         # Because dicts iterate for keys only; this works to get dr names
         return self._distroseries.keys()
@@ -134,12 +132,6 @@
                 'No Lucille config section for %s in %s' %
                     (dr, self.distroName))
 
-    def _extractConfigInfo(self):
-        """Extract configuration information into the attributes we use"""
-        self.stayofexecution = self._distroconfig.get(
-            "publishing", "pendingremovalduration", 5)
-        self.stayofexecution = float(self.stayofexecution)
-
     def setupArchiveDirs(self):
         """Create missing required directories in archive.
 

=== modified file 'lib/lp/archivepublisher/deathrow.py'
--- lib/lp/archivepublisher/deathrow.py	2010-10-08 10:08:48 +0000
+++ lib/lp/archivepublisher/deathrow.py	2010-10-17 12:56:07 +0000
@@ -202,7 +202,6 @@
             if pub.scheduleddeletiondate is None:
                 return False
             # Deny removal if any reference is still in 'quarantine'.
-            # See PubConfig.pendingremovalduration value.
             if pub.scheduleddeletiondate > right_now:
                 return False
 

=== modified file 'lib/lp/archivepublisher/domination.py'
--- lib/lp/archivepublisher/domination.py	2010-10-03 15:30:06 +0000
+++ lib/lp/archivepublisher/domination.py	2010-10-17 12:56:07 +0000
@@ -44,7 +44,7 @@
 The 'domination' procedure is the 2nd step of the publication pipeline and
 it is performed for each suite using:
 
-  * judgeAndDominate(distroseries, pocket, pubconfig)
+  * judgeAndDominate(distroseries, pocket)
 
 """
 
@@ -73,6 +73,10 @@
     )
 
 
+# Days before a package will be removed from disk.
+STAY_OF_EXECUTION = 1
+
+
 def clear_cache():
     """Flush SQLObject updates and clear the cache."""
     # Flush them anyway, should basically be a noop thanks to not doing
@@ -159,7 +163,7 @@
 
         return outpkgs
 
-    def _setScheduledDeletionDate(self, pub_record, conf):
+    def _setScheduledDeletionDate(self, pub_record):
         """Set the scheduleddeletiondate on a publishing record.
 
         If the status is DELETED we set the date to UTC_NOW, otherwise
@@ -169,9 +173,9 @@
             pub_record.scheduleddeletiondate = UTC_NOW
         else:
             pub_record.scheduleddeletiondate = (
-                UTC_NOW + timedelta(days=conf.stayofexecution))
+                UTC_NOW + timedelta(days=STAY_OF_EXECUTION))
 
-    def _judgeSuperseded(self, source_records, binary_records, conf):
+    def _judgeSuperseded(self, source_records, binary_records):
         """Determine whether the superseded packages supplied should
         be moved to death row or not.
 
@@ -221,7 +225,7 @@
                        (binpkg_release.binarypackagename.name,
                         binpkg_release.version,
                         pub_record.distroarchseries.architecturetag))
-            self._setScheduledDeletionDate(pub_record, conf)
+            self._setScheduledDeletionDate(pub_record)
             # XXX cprov 20070820: 'datemadepending' is useless, since it's
             # always equals to "scheduleddeletiondate - quarantine".
             pub_record.datemadepending = UTC_NOW
@@ -268,12 +272,12 @@
                 "%s/%s (%s) source has been judged eligible for removal" %
                 (srcpkg_release.sourcepackagename.name,
                  srcpkg_release.version, pub_record.id))
-            self._setScheduledDeletionDate(pub_record, conf)
+            self._setScheduledDeletionDate(pub_record)
             # XXX cprov 20070820: 'datemadepending' is pointless, since it's
             # always equals to "scheduleddeletiondate - quarantine".
             pub_record.datemadepending = UTC_NOW
 
-    def judgeAndDominate(self, dr, pocket, config, do_clear_cache=True):
+    def judgeAndDominate(self, dr, pocket, do_clear_cache=True):
         """Perform the domination and superseding calculations
 
         It only works across the distroseries and pocket specified.
@@ -366,7 +370,7 @@
                             ELIGIBLE_DOMINATION_STATES),
             clauseTables=['DistroArchSeries'])
 
-        self._judgeSuperseded(sources, binaries, config)
+        self._judgeSuperseded(sources, binaries)
 
         self.debug("Domination for %s/%s finished" %
                    (dr.name, pocket.title))

=== modified file 'lib/lp/archivepublisher/publishing.py'
--- lib/lp/archivepublisher/publishing.py	2010-10-17 12:56:03 +0000
+++ lib/lp/archivepublisher/publishing.py	2010-10-17 12:56:07 +0000
@@ -278,7 +278,7 @@
                                    (distroseries.name, pocket.name))
                         continue
                     self.checkDirtySuiteBeforePublishing(distroseries, pocket)
-                judgejudy.judgeAndDominate(distroseries, pocket, self._config)
+                judgejudy.judgeAndDominate(distroseries, pocket)
 
     def C_doFTPArchive(self, is_careful):
         """Does the ftp-archive step: generates Sources and Packages."""

=== modified file 'lib/lp/archivepublisher/tests/publisher-config.txt'
--- lib/lp/archivepublisher/tests/publisher-config.txt	2010-10-17 12:56:03 +0000
+++ lib/lp/archivepublisher/tests/publisher-config.txt	2010-10-17 12:56:07 +0000
@@ -7,7 +7,6 @@
 We will use a helper function for dumping publisher configurations.
 
     >>> config_attributes = [
-    ...     'stayofexecution',
     ...     'distroroot',
     ...     'archiveroot',
     ...     'poolroot',
@@ -34,7 +33,6 @@
     >>> primary_config = getPubConfig(ubuntutest.main_archive)
 
     >>> dump_config(primary_config)
-    stayofexecution: 5.0
     distroroot:      /var/tmp/archive
     archiveroot:     /var/tmp/archive/ubuntutest
     poolroot:        /var/tmp/archive/ubuntutest/pool
@@ -79,7 +77,6 @@
 None, so they won't get created:
 
     >>> dump_config(ppa_config)
-    stayofexecution: 5.0
     distroroot:      /var/tmp/ppa.test/
     archiveroot:     /var/tmp/ppa.test/cprov/ppa/ubuntutest
     poolroot:        /var/tmp/ppa.test/cprov/ppa/ubuntutest/pool
@@ -110,7 +107,6 @@
     True
 
     >>> dump_config(p3a_config)
-    stayofexecution: 5.0
     distroroot:      /var/tmp/ppa
     archiveroot:     /var/tmp/ppa/cprov/myprivateppa/ubuntutest
     poolroot:        /var/tmp/ppa/cprov/myprivateppa/ubuntutest/pool
@@ -136,7 +132,6 @@
     >>> partner_config = getPubConfig(partner_archive)
 
     >>> dump_config(partner_config)
-    stayofexecution: 5.0
     distroroot:      /var/tmp/archive
     archiveroot:     /var/tmp/archive/ubuntutest-partner
     poolroot:        /var/tmp/archive/ubuntutest-partner/pool
@@ -163,7 +158,6 @@
     >>> debug_config = getPubConfig(debug_archive)
 
     >>> dump_config(debug_config)
-    stayofexecution: 5.0
     distroroot:      /var/tmp/archive
     archiveroot:     /var/tmp/archive/ubuntutest-debug
     poolroot:        /var/tmp/archive/ubuntutest-debug/pool
@@ -188,7 +182,6 @@
     >>> copy_config = getPubConfig(copy_archive)
 
     >>> dump_config(copy_config)
-    stayofexecution: 5.0
     distroroot:      /var/tmp/archive
     archiveroot:     /var/tmp/archive/ubuntutest-rebuildtest99/ubuntutest
     poolroot:        /var/tmp/archive/ubuntutest-rebuildtest99/ubuntutest/pool

=== modified file 'lib/lp/archivepublisher/tests/test_config.py'
--- lib/lp/archivepublisher/tests/test_config.py	2010-10-17 12:56:03 +0000
+++ lib/lp/archivepublisher/tests/test_config.py	2010-10-17 12:56:07 +0000
@@ -50,9 +50,3 @@
         self.assertEquals(len(dsns), 2)
         self.assertEquals(dsns[0], "breezy-autotest")
         self.assertEquals(dsns[1], "hoary-test")
-
-    def testDistroConfig(self):
-        """Config should have parsed a distro config"""
-        d = Config(self.ubuntutest)
-        # NOTE: Add checks here when you add stuff in util.py
-        self.assertEquals(d.stayofexecution, 5)

=== modified file 'lib/lp/archivepublisher/tests/test_dominator.py'
--- lib/lp/archivepublisher/tests/test_dominator.py	2010-08-24 15:29:01 +0000
+++ lib/lp/archivepublisher/tests/test_dominator.py	2010-10-17 12:56:07 +0000
@@ -8,7 +8,7 @@
 import datetime
 
 from canonical.database.sqlbase import flush_database_updates
-from lp.archivepublisher.domination import Dominator
+from lp.archivepublisher.domination import Dominator, STAY_OF_EXECUTION
 from lp.archivepublisher.publishing import Publisher
 from lp.registry.interfaces.series import SeriesStatus
 from lp.soyuz.enums import PackagePublishingStatus
@@ -96,7 +96,7 @@
 
         dominator = Dominator(self.logger, foo_10_source.archive)
         dominator.judgeAndDominate(
-            foo_10_source.distroseries, foo_10_source.pocket, self.config)
+            foo_10_source.distroseries, foo_10_source.pocket)
 
         self.checkPublications(
             [foo_12_source] + foo_12_binaries,
@@ -126,7 +126,7 @@
 
         dominator = Dominator(self.logger, ppa)
         dominator.judgeAndDominate(
-            foo_10_source.distroseries, foo_10_source.pocket, self.config)
+            foo_10_source.distroseries, foo_10_source.pocket)
 
         self.checkPublications(
             [foo_12_source] + foo_12_binaries,
@@ -172,10 +172,6 @@
             status=PackagePublishingStatus.OBSOLETE)
         self.assertTrue(obsoleted_source.scheduleddeletiondate is None)
 
-        # Ensure the stay of execution is 5 days.  This is so that we
-        # can do a sensible check later (see comment below).
-        publisher._config.stayofexecution = 5
-
         publisher.B_dominate(True)
 
         # The publishing records will be scheduled for removal.
@@ -194,7 +190,7 @@
             superseded_source, PackagePublishingStatus.SUPERSEDED)
         self.checkPastDate(
             superseded_source.scheduleddeletiondate,
-            lag=datetime.timedelta(days=publisher._config.stayofexecution))
+            lag=datetime.timedelta(days=STAY_OF_EXECUTION))
 
 
 class TestDominationOfObsoletedSeries(TestDomination):