launchpad-reviewers team mailing list archive

["Jeroen T. Vermeulen" <jtv@xxxxxxxxxxxxx>]

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

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #780214 in Launchpad itself: "Automatic index creation does not set up suite directories in suite"
  https://bugs.launchpad.net/launchpad/+bug/780214

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

= Summary =

When the new, python-based publish-ftpmaster detects that a distroseries still needs archive indexes created for some or all of its suites, it runs publish-distro.

That script, however, requires a directory for the suite to exist in the distsroot.


== Proposed fix ==

Create a directory for the suite to be indexed.


== Pre-implementation notes ==

Discussed quickly with Julian: it's publish-ftpmaster's responsibility, not publish-distro's, to create the directory.


== Implementation details ==

As you'll notice in the test, createIndexes now needs the script's configs dict to be set up.  No big deal, since this will always be the case during script runs; but it broke a test that neglected to call the script's setUp first.


== Tests ==

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


== Demo and Q/A ==

Create a distroseries on dogfood.  Set its status to Frozen.  Run cronscripts/publish-ftpmaster.py on it ("-d <distro>").  It will create indexes in <distsroot>/<suite>, and leave ".created-indexes-for-<suite>" marker files in the archive root.



= 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-780214/+merge/60444
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~jtv/launchpad/bug-780214 into lp:launchpad.

=== modified file 'lib/lp/archivepublisher/scripts/publish_ftpmaster.py'
--- lib/lp/archivepublisher/scripts/publish_ftpmaster.py	2011-05-09 08:48:12 +0000
+++ lib/lp/archivepublisher/scripts/publish_ftpmaster.py	2011-05-10 00:32:36 +0000
@@ -279,6 +279,8 @@
     def createIndexes(self, suite):
         """Create archive indexes for `distroseries`."""
         self.logger.info("Creating archive indexes for %s.", suite)
+        archive_config = self.configs[ArchivePurpose.PRIMARY]
+        os.makedirs(os.path.join(archive_config.distsroot, suite))
         self.runPublishDistro(args=['-A'], suites=[suite])
         self.markIndexCreationComplete(suite)
 

=== modified file 'lib/lp/archivepublisher/tests/test_publish_ftpmaster.py'
--- lib/lp/archivepublisher/tests/test_publish_ftpmaster.py	2011-05-09 10:47:26 +0000
+++ lib/lp/archivepublisher/tests/test_publish_ftpmaster.py	2011-05-10 00:32:36 +0000
@@ -860,6 +860,7 @@
         distro = self.makeDistroWithPublishDirectory()
         series = self.factory.makeDistroSeries(distribution=distro)
         script = self.makeScript(distro)
+        script.setUp()
         script.markIndexCreationComplete = FakeMethod()
         script.runPublishDistro = FakeMethod()
         suite = get_a_suite(series)
@@ -958,6 +959,17 @@
         self.assertEqual([suite], kwargs['suites'])
         self.assertThat(kwargs['suites'][0], StartsWith(series.name))
 
+    def test_createIndexes_creates_suite_directory(self):
+        # When creating indexes, publish-distro requires a directory for
+        # the suite to exist in distsroot.
+        series = self.makeDistroSeriesNeedingIndexes()
+        script = self.makeScript(series.distribution)
+        script.setUp()
+        suite = get_a_suite(series)
+        script.createIndexes(suite)
+        self.assertTrue(file_exists(os.path.join(
+            script.configs[ArchivePurpose.PRIMARY].distsroot, suite)))
+
     def test_script_creates_indexes(self):
         # End-to-end test: the script creates indexes for distroseries
         # that need them.