launchpad-reviewers team mailing list archive

[Jelmer Vernooij <jelmer@xxxxxxxxx>]

Jelmer Vernooij has proposed merging lp:~jelmer/launchpad/55288-publisher-unknown-distroseries into lp:launchpad/devel.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers): code
Related bugs:
  #55288 publisher gets very unhappy about unknown distroreleases
  https://bugs.launchpad.net/bugs/55288


This makes the publisher more robust against distroseries that have not been initialized for soyuz yet. 

Rather than having the publisher configuration raise a LucilleConfigError when a distroseries doesn't have a matching entry in the lucille configuration, it will now write a warning to the log file.
-- 
https://code.launchpad.net/~jelmer/launchpad/55288-publisher-unknown-distroseries/+merge/32232
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~jelmer/launchpad/55288-publisher-unknown-distroseries into lp:launchpad/devel.

=== modified file 'lib/lp/archivepublisher/config.py'
--- lib/lp/archivepublisher/config.py	2010-07-07 06:28:03 +0000
+++ lib/lp/archivepublisher/config.py	2010-08-10 17:43:45 +0000
@@ -120,18 +120,15 @@
                 config_segment["archtags"].append(
                     dar.architecturetag.encode('utf-8'))
 
-            if not dr.lucilleconfig:
-                raise LucilleConfigError(
-                    'No Lucille configuration section for %s' % dr.name)
-
-            strio = StringIO(dr.lucilleconfig.encode('utf-8'))
-            config_segment["config"] = ConfigParser()
-            config_segment["config"].readfp(strio)
-            strio.close()
-            config_segment["components"] = config_segment["config"].get(
-                "publishing", "components").split(" ")
-
-            self._distroseries[distroseries_name] = config_segment
+            if dr.lucilleconfig:
+                strio = StringIO(dr.lucilleconfig.encode('utf-8'))
+                config_segment["config"] = ConfigParser()
+                config_segment["config"].readfp(strio)
+                strio.close()
+                config_segment["components"] = config_segment["config"].get(
+                    "publishing", "components").split(" ")
+
+                self._distroseries[distroseries_name] = config_segment
 
         strio = StringIO(distribution.lucilleconfig.encode('utf-8'))
         self._distroconfig = ConfigParser()
@@ -144,11 +141,19 @@
         # Because dicts iterate for keys only; this works to get dr names
         return self._distroseries.keys()
 
+    def series(self, dr):
+        try:
+            return self._distroseries[dr]
+        except KeyError:
+            raise LucilleConfigError(
+                'No Lucille config section for %s in %s' %
+                    (dr, self.distroName))
+
     def archTagsForSeries(self, dr):
-        return self._distroseries[dr]["archtags"]
+        return self.series(dr)["archtags"]
 
     def componentsForSeries(self, dr):
-        return self._distroseries[dr]["components"]
+        return self.series(dr)["components"]
 
     def _extractConfigInfo(self):
         """Extract configuration information into the attributes we use"""

=== modified file 'lib/lp/archivepublisher/ftparchive.py'
--- lib/lp/archivepublisher/ftparchive.py	2009-10-26 18:40:04 +0000
+++ lib/lp/archivepublisher/ftparchive.py	2010-08-10 17:43:45 +0000
@@ -138,6 +138,14 @@
         self._config = config
         self._diskpool = diskpool
         self.distro = distro
+        self.distroseries = []
+        for distroseries in self.distro.series:
+            if not distroseries.name in self._config.distroSeriesNames():
+                self.log.warning("Distroseries %s in %s doesn't have "
+                    "a lucille configuration.", distroseries.name,
+                    self.distro.name)
+            else:
+                self.distroseries.append(distroseries)
         self.publisher = publisher
         self.release_files_needed = {}
 
@@ -185,7 +193,7 @@
         # iterate over the pockets, and do the suffix check inside
         # createEmptyPocketRequest; that would also allow us to replace
         # the == "" check we do there by a RELEASE match
-        for distroseries in self.distro:
+        for distroseries in self.distroseries:
             components = self._config.componentsForSeries(distroseries.name)
             for pocket, suffix in pocketsuffix.items():
                 if not fullpublish:
@@ -366,7 +374,7 @@
 
     def generateOverrides(self, fullpublish=False):
         """Collect packages that need overrides, and generate them."""
-        for distroseries in self.distro.series:
+        for distroseries in self.distroseries:
             for pocket in PackagePublishingPocket.items:
                 if not fullpublish:
                     if not self.publisher.isDirty(distroseries, pocket):
@@ -629,7 +637,7 @@
 
     def generateFileLists(self, fullpublish=False):
         """Collect currently published FilePublishings and write filelists."""
-        for distroseries in self.distro.series:
+        for distroseries in self.distroseries:
             for pocket in pocketsuffix:
                 if not fullpublish:
                     if not self.publisher.isDirty(distroseries, pocket):

=== modified file 'lib/lp/archivepublisher/tests/test_config.py'
--- lib/lp/archivepublisher/tests/test_config.py	2010-07-18 00:24:06 +0000
+++ lib/lp/archivepublisher/tests/test_config.py	2010-08-10 17:43:45 +0000
@@ -5,36 +5,48 @@
 
 __metaclass__ = type
 
-import unittest
-
 from zope.component import getUtility
 
 from canonical.config import config
 from canonical.launchpad.interfaces import IDistributionSet
 from canonical.testing import LaunchpadZopelessLayer
 
-
-class TestConfig(unittest.TestCase):
+from lp.archivepublisher.config import Config, LucilleConfigError
+from lp.testing import TestCaseWithFactory
+
+
+class TestConfig(TestCaseWithFactory):
     layer = LaunchpadZopelessLayer
 
     def setUp(self):
+        super(TestConfig, self).setUp()
         self.layer.switchDbUser(config.archivepublisher.dbuser)
         self.ubuntutest = getUtility(IDistributionSet)['ubuntutest']
 
+    def testMissingDistroSeries(self):
+        distroseries = self.factory.makeDistroSeries(
+            distribution=self.ubuntutest, name="somename")
+        d = Config(self.ubuntutest)
+        dsns = d.distroSeriesNames()
+        self.assertEquals(len(dsns), 2)
+        self.assertEquals(dsns[0], "breezy-autotest")
+        self.assertEquals(dsns[1], "hoary-test")
+        self.assertRaises(LucilleConfigError,
+            d.archTagsForSeries, "somename")
+        self.assertRaises(LucilleConfigError,
+            d.archTagsForSeries, "unknown")
+
     def testInstantiate(self):
         """Config should instantiate"""
-        from lp.archivepublisher.config import Config
         d = Config(self.ubuntutest)
 
     def testDistroName(self):
         """Config should be able to return the distroName"""
-        from lp.archivepublisher.config import Config
         d = Config(self.ubuntutest)
         self.assertEqual(d.distroName, "ubuntutest")
 
     def testDistroSeriesNames(self):
         """Config should return two distroseries names"""
-        from lp.archivepublisher.config import Config
         d = Config(self.ubuntutest)
         dsns = d.distroSeriesNames()
         self.assertEquals(len(dsns), 2)
@@ -43,14 +55,12 @@
 
     def testArchTagsForSeries(self):
         """Config should have the arch tags for the drs"""
-        from lp.archivepublisher.config import Config
         d = Config(self.ubuntutest)
         archs = d.archTagsForSeries("hoary-test")
         self.assertEquals(len(archs), 2)
 
     def testDistroConfig(self):
         """Config should have parsed a distro config"""
-        from lp.archivepublisher.config import 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_ftparchive.py'
--- lib/lp/archivepublisher/tests/test_ftparchive.py	2010-07-18 00:24:06 +0000
+++ lib/lp/archivepublisher/tests/test_ftparchive.py	2010-08-10 17:43:45 +0000
@@ -15,7 +15,7 @@
 from zope.component import getUtility
 
 from canonical.config import config
-from canonical.launchpad.scripts.logger import QuietFakeLogger
+from canonical.launchpad.scripts.logger import BufferLogger, QuietFakeLogger
 from canonical.testing import LaunchpadZopelessLayer
 from lp.archivepublisher.config import Config
 from lp.archivepublisher.diskpool import DiskPool
@@ -23,6 +23,7 @@
 from lp.archivepublisher.publishing import Publisher
 from lp.registry.interfaces.distribution import IDistributionSet
 from lp.registry.interfaces.pocket import PackagePublishingPocket
+from lp.testing import TestCaseWithFactory
 
 
 def sanitize_apt_ftparchive_Sources_output(text):
@@ -55,10 +56,11 @@
         return self._result[i:j]
 
 
-class TestFTPArchive(unittest.TestCase):
+class TestFTPArchive(TestCaseWithFactory):
     layer = LaunchpadZopelessLayer
 
     def setUp(self):
+        super(TestFTPArchive, self).setUp()
         self.layer.switchDbUser(config.archivepublisher.dbuser)
 
         self._distribution = getUtility(IDistributionSet)['ubuntutest']
@@ -79,6 +81,7 @@
         self._publisher = SamplePublisher(self._archive)
 
     def tearDown(self):
+        super(TestFTPArchive, self).tearDown()
         shutil.rmtree(self._config.distroroot)
 
     def _verifyFile(self, filename, directory, output_filter=None):
@@ -116,6 +119,19 @@
             self._publisher)
         return fa
 
+    def test_NoLucilleConfig(self):
+        # Distroseries without a lucille configuration get ignored
+        # and trigger a warning, they don't break the publisher
+        logger = BufferLogger()
+        publisher = Publisher(
+            logger, self._config, self._dp, self._archive)
+        self.factory.makeDistroSeries(self._distribution, name="somename")
+        fa = FTPArchiveHandler(logger, self._config, self._dp,
+                               self._distribution, publisher)
+        fa.createEmptyPocketRequests(fullpublish=True)
+        self.assertEquals("WARNING: Distroseries somename in ubuntutest doesn't "
+            "have a lucille configuration.\n", logger.buffer.getvalue())
+
     def test_getSourcesForOverrides(self):
         # getSourcesForOverrides returns a list of tuples containing:
         # (sourcename, suite, component, section)