← Back to team overview

launchpad-reviewers team mailing list archive

[Merge] lp:~jtv/launchpad/bug-779701 into lp:launchpad

 

Jeroen T. Vermeulen has proposed merging lp:~jtv/launchpad/bug-779701 into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #779701 in Launchpad itself: "Initialize new distroseries' indexes separately per suite"
  https://bugs.launchpad.net/launchpad/+bug/779701

For more details, see:
https://code.launchpad.net/~jtv/launchpad/bug-779701/+merge/60346

The new, python-based publish-ftpmaster script will create archive indexes for a new distroseries.  In the Ubuntu release process this removes a manual "publish-distro.py -A" run for the distroseries.  However we want the indexes for each suite in the series created separately, with a separate publish-distro run.  This will make it easier to add pockets in the future, for instance.  Index creation should be tracked separately for each suite.

This branch makes the index-creation code use the suite name instead of the distroseries name for the marker files that indicate that indexes have been created.  It then runs publish-distro separately for each suite.

To test:
{{{
./bin/test -vvc lp.archivepublisher.tests.test_publish_ftpmaster
}}}

To Q/A:
 * Create a new distroseries for an existing distribution.
 * Run cronscripts/publish-ftpmaster.py -d <distro> -s <series>.
 * See that indexes have been created.
 * Note marker files in the distribution's archive root, one for each suites.

(The marker files are dot files, so may not show up by default).


= Launchpad lint =

Checking for conflicts and issues in changed files.

Linting changed files:
  lib/lp/archivepublisher/scripts/publish_ftpmaster.py
  lib/lp/archivepublisher/tests/test_publish_ftpmaster.py
-- 
https://code.launchpad.net/~jtv/launchpad/bug-779701/+merge/60346
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~jtv/launchpad/bug-779701 into lp:launchpad.
=== modified file 'lib/lp/archivepublisher/scripts/publish_ftpmaster.py'
--- lib/lp/archivepublisher/scripts/publish_ftpmaster.py	2011-05-06 03:10:14 +0000
+++ lib/lp/archivepublisher/scripts/publish_ftpmaster.py	2011-05-09 09:06:05 +0000
@@ -228,19 +228,18 @@
             (archive.purpose, getPubConfig(archive))
             for archive in self.archives)
 
-    def locateIndexesMarker(self, distroseries):
+    def locateIndexesMarker(self, suite):
         """Give path for marker file whose presence marks index creation.
 
-        The file will be created once the archive indexes for
-        `distroseries` have been created.  This is how future runs will
-        know that this work is done.
+        The file will be created once the archive indexes for `suite`
+        have been created.  This is how future runs will know that this
+        work is done.
         """
         archive_root = self.configs[ArchivePurpose.PRIMARY].archiveroot
-        return os.path.join(
-            archive_root, ".created-indexes-for-%s" % distroseries.name)
+        return os.path.join(archive_root, ".created-indexes-for-%s" % suite)
 
-    def needsIndexesCreated(self, distroseries):
-        """Does `distroseries` still need its archive indexes created?
+    def listSuitesNeedingIndexes(self, distroseries):
+        """Find suites in `distroseries` that need indexes created.
 
         Checks for the marker left by `markIndexCreationComplete`.
         """
@@ -249,36 +248,39 @@
             # is in any other state yet has not been marked as having
             # its indexes created, that's because it predates automatic
             # index creation.
-            return False
+            return []
         distro = distroseries.distribution
         publisher_config_set = getUtility(IPublisherConfigSet)
         if publisher_config_set.getByDistribution(distro) is None:
             # We won't be able to do a thing without a publisher config,
             # but that's alright: we have those for all distributions
             # that we want to publish.
-            return False
-        return not file_exists(self.locateIndexesMarker(distroseries))
-
-    def markIndexCreationComplete(self, distroseries):
-        """Note that archive indexes for `distroseries` have been created.
-
-        This tells `needsIndexesCreated` that no, this series no longer needs
-        archive indexes to be set up.
+            return []
+
+        # May need indexes for this series.
+        suites = [
+            distroseries.getSuite(pocket)
+            for pocket in pocketsuffix.iterkeys()]
+        return [
+            suite for suite in suites
+                if not file_exists(self.locateIndexesMarker(suite))]
+
+    def markIndexCreationComplete(self, suite):
+        """Note that archive indexes for `suite` have been created.
+
+        This tells `listSuitesNeedingIndexes` that no, this suite no
+        longer needs archive indexes to be set up.
         """
-        with file(self.locateIndexesMarker(distroseries), "w") as marker:
+        with file(self.locateIndexesMarker(suite), "w") as marker:
             marker.write(
                 "Indexes for %s were created on %s.\n"
-                % (distroseries, datetime.now(utc)))
+                % (suite, datetime.now(utc)))
 
-    def createIndexes(self, distroseries):
+    def createIndexes(self, suite):
         """Create archive indexes for `distroseries`."""
-        self.logger.info(
-            "Creating archive indexes for series %s.", distroseries)
-        suites = [
-            distroseries.getSuite(pocket)
-            for pocket in pocketsuffix.iterkeys()]
-        self.runPublishDistro(args=['-A'], suites=suites)
-        self.markIndexCreationComplete(distroseries)
+        self.logger.info("Creating archive indexes for %s.", suite)
+        self.runPublishDistro(args=['-A'], suites=[suite])
+        self.markIndexCreationComplete(suite)
 
     def processAccepted(self):
         """Run the process-accepted script."""
@@ -556,8 +558,10 @@
         self.recoverWorkingDists()
 
         for series in self.distribution.series:
-            if self.needsIndexesCreated(series):
-                self.createIndexes(series)
+            suites_needing_indexes = self.listSuitesNeedingIndexes(series)
+            if len(suites_needing_indexes) > 0:
+                for suite in suites_needing_indexes:
+                    self.createIndexes(suite)
                 # Don't try to do too much in one run.  Leave the rest
                 # of the work for next time.
                 return

=== modified file 'lib/lp/archivepublisher/tests/test_publish_ftpmaster.py'
--- lib/lp/archivepublisher/tests/test_publish_ftpmaster.py	2011-05-06 03:10:14 +0000
+++ lib/lp/archivepublisher/tests/test_publish_ftpmaster.py	2011-05-09 09:06:05 +0000
@@ -108,6 +108,19 @@
     return file(os.path.join(*path)).read()
 
 
+def get_a_suite(distroseries):
+    """Return some suite name for `distroseries`."""
+    # Don't pick Release; it's too easy.
+    return distroseries.getSuite(PackagePublishingPocket.SECURITY)
+
+
+def get_marker_files(script, distroseries):
+        suites = [
+            distroseries.getSuite(pocket)
+            for pocket in pocketsuffix.iterkeys()]
+        return [script.locateIndexesMarker(suite) for suite in suites]
+
+
 class HelpersMixin:
     """Helpers for the PublishFTPMaster tests."""
 
@@ -769,36 +782,49 @@
 
     def createIndexesMarkerDir(self, script, distroseries):
         """Create the directory for `distroseries`'s indexes marker."""
-        marker = script.locateIndexesMarker(distroseries)
+        marker = script.locateIndexesMarker(get_a_suite(distroseries))
         os.makedirs(os.path.dirname(marker))
 
-    def test_new_frozen_series_needs_indexes_created(self):
+    def test_new_frozen_series_has_suites_without_indexes(self):
         # If a distroseries is Frozen and has not had its indexes
-        # created yet, needsIndexesCreated returns True for it.
-        series = self.factory.makeDistroSeries(status=SeriesStatus.FROZEN)
-        script = self.makeScript(series.distribution)
-        script.setUp()
-        self.assertTrue(script.needsIndexesCreated(series))
+        # created yet, listSuitesNeedingIndexes returns a nonempty list
+        # for it.
+        series = self.factory.makeDistroSeries(status=SeriesStatus.FROZEN)
+        script = self.makeScript(series.distribution)
+        script.setUp()
+        self.assertNotEqual([], list(script.listSuitesNeedingIndexes(series)))
+
+    def test_listSuitesNeedingIndexes_initially_includes_entire_series(self):
+        # If a series has not had any of its indexes created yet,
+        # listSuitesNeedingIndexes returns all of its suites.
+        series = self.factory.makeDistroSeries(status=SeriesStatus.FROZEN)
+        script = self.makeScript(series.distribution)
+        script.setUp()
+        self.assertContentEqual(
+            [series.getSuite(pocket) for pocket in pocketsuffix.iterkeys()],
+            script.listSuitesNeedingIndexes(series))
 
     def test_new_nonfrozen_series_does_not_need_indexes_created(self):
-        # needsIndexesCreated only returns True for Frozen distroseries.
+        # listSuitesNeedingIndexes only returns suites for Frozen
+        # distroseries.
         series = self.factory.makeDistroSeries()
         script = self.makeScript(series.distribution)
-        self.assertFalse(script.needsIndexesCreated(series))
+        self.assertEqual([], script.listSuitesNeedingIndexes(series))
 
     def test_distro_without_publisher_config_gets_no_indexes(self):
-        # needsIndexesCreated returns False for distributions that have
-        # no publisher config, such as Debian.  We don't want to publish
-        # these distributions.
+        # listSuitesNeedingIndexes returns no suites for distributions
+        # that have no publisher config, such as Debian.  We don't want
+        # to publish such distributions.
         series = self.factory.makeDistroSeries(status=SeriesStatus.FROZEN)
         pub_config = get_pub_config(series.distribution)
         IMasterStore(pub_config).remove(pub_config)
         script = self.makeScript(series.distribution)
-        self.assertFalse(script.needsIndexesCreated(series))
+        self.assertEqual([], script.listSuitesNeedingIndexes(series))
 
-    def test_markIndexCreationComplete_tells_needsIndexesCreated_no(self):
-        # The effect of markIndexCreationComplete is to make
-        # needsIndexesCreated for that distroseries return False.
+    def test_markIndexCreationComplete_repels_listSuitesNeedingIndexes(self):
+        # The effect of markIndexCreationComplete is to remove the suite
+        # in question from the results of listSuitesNeedingIndexes for
+        # that distroseries.
         distro = self.makeDistroWithPublishDirectory()
         series = self.factory.makeDistroSeries(
             status=SeriesStatus.FROZEN, distribution=distro)
@@ -806,21 +832,36 @@
         script.setUp()
         self.createIndexesMarkerDir(script, series)
 
-        self.assertTrue(script.needsIndexesCreated(series))
-        script.markIndexCreationComplete(series)
-        self.assertFalse(script.needsIndexesCreated(series))
+        needful_suites = script.listSuitesNeedingIndexes(series)
+        suite = get_a_suite(series)
+        script.markIndexCreationComplete(suite)
+        needful_suites.remove(suite)
+        self.assertContentEqual(
+            needful_suites, script.listSuitesNeedingIndexes(series))
+
+    def test_listSuitesNeedingIndexes_ignores_other_series(self):
+        # listSuitesNeedingIndexes only returns suites for series that
+        # need indexes created.  It ignores other distroseries.
+        series = self.factory.makeDistroSeries(status=SeriesStatus.FROZEN)
+        self.factory.makeDistroSeries(distribution=series.distribution)
+        script = self.makeScript(series.distribution)
+        script.setUp()
+        suites = list(script.listSuitesNeedingIndexes(series))
+        self.assertNotEqual([], suites)
+        for suite in suites:
+            self.assertThat(suite, StartsWith(series.name))
 
     def test_createIndexes_marks_index_creation_complete(self):
-        # createIndexes calls markIndexCreationComplete for the
-        # distroseries.
+        # createIndexes calls markIndexCreationComplete for the suite.
         distro = self.makeDistroWithPublishDirectory()
         series = self.factory.makeDistroSeries(distribution=distro)
         script = self.makeScript(distro)
         script.markIndexCreationComplete = FakeMethod()
         script.runPublishDistro = FakeMethod()
-        script.createIndexes(series)
+        suite = get_a_suite(series)
+        script.createIndexes(suite)
         self.assertEqual(
-            [((series, ), {})], script.markIndexCreationComplete.calls)
+            [((suite, ), {})], script.markIndexCreationComplete.calls)
 
     def test_failed_index_creation_is_not_marked_complete(self):
         # If index creation fails, it is not marked as having been
@@ -833,7 +874,7 @@
         script.markIndexCreationComplete = FakeMethod()
         script.runPublishDistro = FakeMethod(failure=Boom("Sorry!"))
         try:
-            script.createIndexes(series)
+            script.createIndexes(get_a_suite(series))
         except:
             pass
         self.assertEqual([], script.markIndexCreationComplete.calls)
@@ -846,20 +887,30 @@
         script.setUp()
         archive_root = script.configs[ArchivePurpose.PRIMARY].archiveroot
         self.assertThat(
-            script.locateIndexesMarker(series),
+            script.locateIndexesMarker(get_a_suite(series)),
             StartsWith(os.path.normpath(archive_root)))
 
-    def test_locateIndexesMarker_uses_separate_files_per_series(self):
-        # Different release series of the same distribution get separate
-        # marker files for index creation.
+    def test_locateIndexesMarker_uses_separate_files_per_suite(self):
+        # Each suite in a distroseries gets its own marker file for
+        # index creation.
+        distro = self.makeDistroWithPublishDirectory()
+        series = self.factory.makeDistroSeries(distribution=distro)
+        script = self.makeScript(distro)
+        script.setUp()
+        markers = get_marker_files(script, series)
+        self.assertEqual(sorted(markers), sorted(list(set(markers))))
+
+    def test_locateIndexesMarker_separates_distroseries(self):
+        # Each distroseries gets its own marker files for index
+        # creation.
         distro = self.makeDistroWithPublishDirectory()
         series1 = self.factory.makeDistroSeries(distribution=distro)
         series2 = self.factory.makeDistroSeries(distribution=distro)
         script = self.makeScript(distro)
         script.setUp()
-        self.assertNotEqual(
-            script.locateIndexesMarker(series1),
-            script.locateIndexesMarker(series2))
+        markers1 = set(get_marker_files(script, series1))
+        markers2 = set(get_marker_files(script, series2))
+        self.assertEqual(set(), markers1.intersection(markers2))
 
     def test_locateIndexMarker_uses_hidden_file(self):
         # The index-creation marker file is a "dot file," so it's not
@@ -867,8 +918,9 @@
         series = self.factory.makeDistroSeries()
         script = self.makeScript(series.distribution)
         script.setUp()
+        suite = get_a_suite(series)
         self.assertThat(
-            os.path.basename(script.locateIndexesMarker(series)),
+            os.path.basename(script.locateIndexesMarker(suite)),
             StartsWith("."))
 
     def test_script_calls_createIndexes_for_new_series(self):
@@ -880,7 +932,10 @@
         script = self.makeScript(distro)
         script.createIndexes = FakeMethod()
         script.main()
-        self.assertEqual([((series, ), {})], script.createIndexes.calls)
+        expected_calls = [
+            ((series.getSuite(pocket), ), {})
+            for pocket in pocketsuffix.iterkeys()]
+        self.assertContentEqual(expected_calls, script.createIndexes.calls)
 
     def test_createIndexes_ignores_other_series(self):
         # createIndexes does not accidentally also touch other
@@ -892,14 +947,13 @@
         script.setUp()
         script.runPublishDistro = FakeMethod()
         self.createIndexesMarkerDir(script, series)
+        suite = get_a_suite(series)
 
-        script.createIndexes(series)
+        script.createIndexes(suite)
 
         args, kwargs = script.runPublishDistro.calls[0]
-        suites = kwargs['suites']
-        self.assertEqual(len(pocketsuffix), len(suites))
-        for suite in suites:
-            self.assertThat(suite, StartsWith(series.name))
+        self.assertEqual([suite], kwargs['suites'])
+        self.assertThat(kwargs['suites'][0], StartsWith(series.name))
 
     def test_script_creates_indexes(self):
         # End-to-end test: the script creates indexes for distroseries
@@ -913,7 +967,7 @@
         self.setUpForScriptRun(series.distribution)
         script = self.makeScript(series.distribution)
         script.main()
-        self.assertFalse(script.needsIndexesCreated(series))
+        self.assertEqual([], script.listSuitesNeedingIndexes(series))
         sources = os.path.join(
             script.configs[ArchivePurpose.PRIMARY].distsroot,
             series.name, "main", "source", "Sources")


Follow ups