launchpad-reviewers team mailing list archive

[William Grant <me@xxxxxxxxxxxxxxxxxx>]

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

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)


A continuation of my anti-lucilleconfig tirade from lp:~wgrant/launchpad/destroy-distroseries-lucilleconfig, this branch obsoletes Distribution.lucilleconfig's "publishing" section.

getPubConfig has conditionally overridden Config's paths since PPAs were introduced some years ago. This branch moves the calculation out of Config and into getPubConfig, replacing dependency on in-database configuration with values that are already in the LAZR config.

Before this can land, production configs need to be checked. The only change should be that ${archivepublisher.root}/ubuntu-test is now used instead of /srv/launchpad.net/ubuntu-archive/ubuntu-test, but this may make germanium unhappy.
-- 
https://code.launchpad.net/~wgrant/launchpad/replace-lucilleconfig-directories/+merge/38650
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~wgrant/launchpad/replace-lucilleconfig-directories into lp:launchpad/devel.

=== modified file 'lib/lp/archivepublisher/config.py'
--- lib/lp/archivepublisher/config.py	2010-10-17 11:59:46 +0000
+++ lib/lp/archivepublisher/config.py	2010-10-17 11:59:48 +0000
@@ -14,16 +14,7 @@
 from lp.soyuz.enums import ArchivePurpose
 
 
-def update_pub_config(pubconf):
-    """Update dependent `PubConfig` fields.
-
-    Update fields dependending on 'archiveroot'.
-    """
-    pubconf.poolroot = os.path.join(pubconf.archiveroot, 'pool')
-    pubconf.distsroot = os.path.join(pubconf.archiveroot, 'dists')
-    pubconf.overrideroot = None
-    pubconf.cacheroot = None
-    pubconf.miscroot = None
+APT_FTPARCHIVE_PURPOSES = (ArchivePurpose.PRIMARY, ArchivePurpose.COPY)
 
 
 def getPubConfig(archive):
@@ -36,9 +27,10 @@
     pubconf = Config(archive.distribution)
     ppa_config = config.personalpackagearchive
 
-    if archive.purpose == ArchivePurpose.PRIMARY:
-        pass
-    elif archive.is_ppa:
+    pubconf.temproot = os.path.join(
+        config.archivepublisher.root, '%s-temp' % pubconf.distroName)
+
+    if archive.is_ppa:
         if archive.private:
             pubconf.distroroot = ppa_config.private_root
             pubconf.htaccessroot = os.path.join(
@@ -49,34 +41,41 @@
         pubconf.archiveroot = os.path.join(
             pubconf.distroroot, archive.owner.name, archive.name,
             archive.distribution.name)
-        update_pub_config(pubconf)
-    elif archive.purpose == ArchivePurpose.PARTNER:
-        pubconf.distroroot = config.archivepublisher.root
-        pubconf.archiveroot = os.path.join(
-            pubconf.distroroot, archive.distribution.name + '-partner')
-        update_pub_config(pubconf)
-    elif archive.purpose == ArchivePurpose.DEBUG:
-        pubconf.distroroot = config.archivepublisher.root
-        pubconf.archiveroot = os.path.join(
-            pubconf.distroroot, archive.distribution.name + '-debug')
-        update_pub_config(pubconf)
+    elif archive.is_main:
+        pubconf.distroroot = config.archivepublisher.root
+        pubconf.archiveroot = os.path.join(
+            pubconf.distroroot, archive.distribution.name)
+        if archive.purpose == ArchivePurpose.PARTNER:
+            pubconf.archiveroot += '-partner'
+        elif archive.purpose == ArchivePurpose.DEBUG:
+            pubconf.archiveroot += '-debug'
     elif archive.is_copy:
         pubconf.distroroot = config.archivepublisher.root
         pubconf.archiveroot = os.path.join(
             pubconf.distroroot,
             archive.distribution.name + '-' + archive.name,
             archive.distribution.name)
-        # Multiple copy archives can exist on the same machine so the
-        # temp areas need to be unique also.
+    else:
+        raise AssertionError(
+            "Unknown archive purpose %s when getting publisher config.",
+            archive.purpose)
+
+    # There can be multiple copy archives, so the temp dir needs to be
+    # within the archive.
+    if archive.is_copy:
         pubconf.temproot = pubconf.archiveroot + '-temp'
-        update_pub_config(pubconf)
+
+    if archive.purpose in APT_FTPARCHIVE_PURPOSES:
         pubconf.overrideroot = pubconf.archiveroot + '-overrides'
         pubconf.cacheroot = pubconf.archiveroot + '-cache'
         pubconf.miscroot = pubconf.archiveroot + '-misc'
     else:
-        raise AssertionError(
-            "Unknown archive purpose %s when getting publisher config.",
-            archive.purpose)
+        pubconf.overrideroot = None
+        pubconf.cacheroot = None
+        pubconf.miscroot = None
+
+    pubconf.poolroot = os.path.join(pubconf.archiveroot, 'pool')
+    pubconf.distsroot = os.path.join(pubconf.archiveroot, 'dists')
 
     meta_root = os.path.join(
         pubconf.distroroot, archive.owner.name)
@@ -140,20 +139,6 @@
         self.stayofexecution = self._distroconfig.get(
             "publishing", "pendingremovalduration", 5)
         self.stayofexecution = float(self.stayofexecution)
-        self.distroroot = self._distroconfig.get("publishing","root")
-        self.archiveroot = self._distroconfig.get("publishing","archiveroot")
-        self.poolroot = self._distroconfig.get("publishing","poolroot")
-        self.distsroot = self._distroconfig.get("publishing","distsroot")
-        self.overrideroot = self._distroconfig.get(
-            "publishing","overrideroot")
-        self.cacheroot = self._distroconfig.get("publishing","cacheroot")
-        self.miscroot = self._distroconfig.get("publishing","miscroot")
-        # XXX cprov 2007-04-26 bug=45270:
-        # We should build all the previous attributes
-        # dynamically like this. It would reduce the configuration complexity.
-        # Even before we have it properly modeled in LPDB.
-        self.temproot = os.path.join(
-            self.distroroot, '%s-temp' % self.distroName)
 
     def setupArchiveDirs(self):
         """Create missing required directories in archive.

=== modified file 'lib/lp/archivepublisher/tests/test_ftparchive.py'
--- lib/lp/archivepublisher/tests/test_ftparchive.py	2010-10-17 04:13:24 +0000
+++ lib/lp/archivepublisher/tests/test_ftparchive.py	2010-10-17 11:59:48 +0000
@@ -20,7 +20,7 @@
     QuietFakeLogger,
     )
 from canonical.testing.layers import LaunchpadZopelessLayer
-from lp.archivepublisher.config import Config
+from lp.archivepublisher.config import getPubConfig
 from lp.archivepublisher.diskpool import DiskPool
 from lp.archivepublisher.ftparchive import (
     f_touch,
@@ -71,7 +71,7 @@
 
         self._distribution = getUtility(IDistributionSet)['ubuntutest']
         self._archive = self._distribution.main_archive
-        self._config = Config(self._distribution)
+        self._config = getPubConfig(self._archive)
         self._config.setupArchiveDirs()
         self._sampledir = os.path.join(
             config.root, "lib", "lp", "archivepublisher", "tests",

=== modified file 'lib/lp/archivepublisher/tests/test_publisher.py'
--- lib/lp/archivepublisher/tests/test_publisher.py	2010-10-17 04:13:24 +0000
+++ lib/lp/archivepublisher/tests/test_publisher.py	2010-10-17 11:59:48 +0000
@@ -1110,7 +1110,7 @@
         self.ubuntutest.getSeries('breezy-autotest').status = (
             SeriesStatus.FROZEN)
 
-        self.config = Config(self.ubuntutest)
+        self.config = getPubConfig(self.ubuntutest.main_archive)
         publisher = Publisher(
             self.logger, self.config, self.disk_pool,
             self.ubuntutest.main_archive)
@@ -1131,7 +1131,7 @@
 
         ds = self.ubuntutest.getSeries('breezy-autotest')
         ds.getDistroArchSeries('i386').enabled = False
-        self.config = Config(self.ubuntutest)
+        self.config = getPubConfig(self.ubuntutest.main_archive)
 
         publisher = Publisher(
             self.logger, self.config, self.disk_pool,

=== modified file 'lib/lp/soyuz/tests/test_publishing.py'
--- lib/lp/soyuz/tests/test_publishing.py	2010-10-06 11:46:51 +0000
+++ lib/lp/soyuz/tests/test_publishing.py	2010-10-17 11:59:48 +0000
@@ -26,7 +26,7 @@
     )
 from canonical.testing.layers import reconnect_stores
 from lp.app.errors import NotFoundError
-from lp.archivepublisher.config import Config
+from lp.archivepublisher.config import getPubConfig
 from lp.archivepublisher.diskpool import DiskPool
 from lp.buildmaster.enums import BuildStatus
 from lp.registry.interfaces.distribution import IDistributionSet
@@ -571,7 +571,7 @@
         super(TestNativePublishingBase, self).setUp()
         self.layer.switchDbUser(config.archivepublisher.dbuser)
         self.prepareBreezyAutotest()
-        self.config = Config(self.ubuntutest)
+        self.config = getPubConfig(self.ubuntutest.main_archive)
         self.config.setupArchiveDirs()
         self.pool_dir = self.config.poolroot
         self.temp_dir = self.config.temproot