launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #08990
[Merge] lp:~cjwatson/launchpad/generate-contents-direct-query-distro into lp:launchpad
Colin Watson has proposed merging lp:~cjwatson/launchpad/generate-contents-direct-query-distro into lp:launchpad.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/generate-contents-direct-query-distro/+merge/111126
== Summary ==
GenerateContentsFiles already uses the database directly, so there's no need for it to go through the LpQueryDistro script which was written to make it easier to integrate a number of shell scripts which were extant at the time; it's just a waste. This is probably a leftover of the old shell version of this script.
Two of the three remaining actions in LpQueryDistro are used only by this script. The third is also used by cron.germinate; I have a few alternative ideas for getting rid of that use depending on what turns out to be feasible, so one way or another we should be able to get rid of LpQueryDistro entirely in the not too distant future.
== Proposed fix ==
Let's just have it get the information it needs directly. We can further simplify things by getting the list of architectures per series being operated on (which is more accurate anyway) rather than worrying about fetching the development series.
== Tests ==
bin/test -vvct test_generate_contents_files -t test_lpquerydistro
== Demo and Q/A ==
None needed.
== Lint ==
Just the usual false positive for scripts:
./scripts/ftpmaster-tools/lp-query-distro.py
21: '_pythonpath' imported but unused
--
https://code.launchpad.net/~cjwatson/launchpad/generate-contents-direct-query-distro/+merge/111126
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad/generate-contents-direct-query-distro into lp:launchpad.
=== modified file 'lib/lp/archivepublisher/scripts/generate_contents_files.py'
--- lib/lp/archivepublisher/scripts/generate_contents_files.py 2012-01-19 03:09:38 +0000
+++ lib/lp/archivepublisher/scripts/generate_contents_files.py 2012-06-20 01:32:25 +0000
@@ -15,7 +15,8 @@
from lp.archivepublisher.config import getPubConfig
from lp.registry.interfaces.distribution import IDistributionSet
-from lp.registry.interfaces.pocket import pocketsuffix
+from lp.registry.interfaces.pocket import PackagePublishingPocket
+from lp.registry.interfaces.series import SeriesStatus
from lp.services.command_spawner import (
CommandSpawner,
OutputLineHandler,
@@ -31,7 +32,6 @@
DatabaseBlockedPolicy,
SlaveOnlyDatabasePolicy,
)
-from lp.soyuz.scripts.querydistro import LpQueryDistro
COMPONENTS = [
@@ -54,14 +54,6 @@
return False
-class StoreArgument:
- """Local helper for receiving `LpQueryDistro` results."""
-
- def __call__(self, argument):
- """Store call argument."""
- self.argument = argument
-
-
def get_template(template_name):
"""Return path of given template in this script's templates directory."""
return os.path.join(
@@ -150,44 +142,36 @@
if not file_exists(path):
os.makedirs(path)
- def queryDistro(self, request, options=None):
- """Call the query-distro script about `self.distribution`."""
- args = ['-d', self.distribution.name]
- if options is not None:
- args += options
- args.append(request)
- query_distro = LpQueryDistro(test_args=args, logger=self.logger)
- query_distro.txn = self.txn
- receiver = StoreArgument()
- query_distro.runAction(presenter=receiver)
- return receiver.argument
+ def getSupportedSeries(self):
+ """Return suites that are supported in this distribution.
+
+ "Supported" means not EXPERIMENTAL or OBSOLETE.
+ """
+ unsupported_status = (SeriesStatus.EXPERIMENTAL,
+ SeriesStatus.OBSOLETE)
+ for series in self.distribution:
+ if series.status not in unsupported_status:
+ yield series
def getSuites(self):
- """Query the distribution's suites."""
- return self.queryDistro("supported").split()
-
- def getPockets(self):
"""Return suites that are actually supported in this distribution."""
- pockets = []
- pocket_suffixes = pocketsuffix.values()
- for suite in self.getSuites():
- for pocket_suffix in pocket_suffixes:
- pocket = suite + pocket_suffix
- if file_exists(os.path.join(self.config.distsroot, pocket)):
- pockets.append(pocket)
- return pockets
+ for series in self.getSupportedSeries():
+ for pocket in PackagePublishingPocket.items:
+ suite = series.getSuite(pocket)
+ if file_exists(os.path.join(self.config.distsroot, suite)):
+ yield suite
- def getArchs(self):
- """Query architectures supported by the distribution."""
- devel = self.queryDistro("development")
- return self.queryDistro("archs", options=["-s", devel]).split()
+ def getArchs(self, suite):
+ """Query architectures supported by the suite."""
+ series, _ = self.distribution.getDistroSeriesAndPocket(suite)
+ return [arch.architecturetag for arch in series.architectures]
def getDirs(self, archs):
"""Subdirectories needed for each component."""
return ['source', 'debian-installer'] + [
'binary-%s' % arch for arch in archs]
- def writeAptContentsConf(self, suites, archs):
+ def writeAptContentsConf(self, suites):
"""Write apt-contents.conf file."""
output_dirname = '%s-misc' % self.distribution.name
output_path = os.path.join(
@@ -195,7 +179,6 @@
output_file = file(output_path, 'w')
parameters = {
- 'architectures': ' '.join(archs),
'content_archive': self.content_archive,
'distribution': self.distribution.name,
}
@@ -206,15 +189,16 @@
dist_template = file(get_template('apt_conf_dist.template')).read()
for suite in suites:
parameters['suite'] = suite
+ parameters['architectures'] = ' '.join(self.getArchs(suite))
output_file.write(dist_template % parameters)
output_file.close()
- def createComponentDirs(self, suites, archs):
+ def createComponentDirs(self, suites):
"""Create the content archive's tree for all of its components."""
for suite in suites:
for component in COMPONENTS:
- for directory in self.getDirs(archs):
+ for directory in self.getDirs(self.getArchs(suite)):
path = os.path.join(
self.content_archive, self.distribution.name, 'dists',
suite, component, directory)
@@ -309,11 +293,11 @@
self.logger.debug(
"Skipping unmodified Contents file for %s/%s.", suite, arch)
- def updateContentsFiles(self, suites, archs):
+ def updateContentsFiles(self, suites):
"""Update all Contents files that have changed."""
self.logger.debug("Comparing contents files with public tree.")
for suite in suites:
- for arch in archs:
+ for arch in self.getArchs(suite):
self.updateContentsFile(suite, arch)
def setUp(self):
@@ -332,10 +316,9 @@
def process(self):
"""Do the bulk of the work."""
self.setUp()
- suites = self.getPockets()
- archs = self.getArchs()
- self.writeAptContentsConf(suites, archs)
- self.createComponentDirs(suites, archs)
+ suites = list(self.getSuites())
+ self.writeAptContentsConf(suites)
+ self.createComponentDirs(suites)
overrideroot = self.config.overrideroot
distro_name = self.distribution.name
@@ -348,7 +331,7 @@
self.generateContentsFiles(
overrideroot, distro_name, distro_title)
- self.updateContentsFiles(suites, archs)
+ self.updateContentsFiles(suites)
def main(self):
"""See `LaunchpadScript`."""
=== modified file 'lib/lp/archivepublisher/tests/test_generate_contents_files.py'
--- lib/lp/archivepublisher/tests/test_generate_contents_files.py 2012-01-01 02:58:52 +0000
+++ lib/lp/archivepublisher/tests/test_generate_contents_files.py 2012-06-20 01:32:25 +0000
@@ -1,4 +1,4 @@
-# Copyright 2011 Canonical Ltd. This software is licensed under the
+# Copyright 2011-2012 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
"""Test for the `generate-contents-files` script."""
@@ -40,7 +40,7 @@
os.makedirs(script.config.overrideroot)
components = ['main', 'restricted', 'universe', 'multiverse']
- architectures = script.getArchs()
+ architectures = script.getArchs(distroseries.name)
suffixes = components + ['extra.' + component for component in components]
for suffix in suffixes:
write_file(os.path.join(
@@ -201,42 +201,35 @@
script = self.makeScript(distro)
self.assertEqual(distro, script.distribution)
- def test_queryDistro(self):
- # queryDistro is a helper that invokes LpQueryDistro.
- distro = self.makeDistro()
- distroseries = self.factory.makeDistroSeries(distro)
- script = self.makeScript(distro)
- script.processOptions()
- self.assertEqual(distroseries.name, script.queryDistro('supported'))
-
def test_getArchs(self):
- # getArchs returns a list of architectures in the distribution.
+ # getArchs returns a list of architectures in the distroseries.
distro = self.makeDistro()
distroseries = self.factory.makeDistroSeries(distro)
das = self.factory.makeDistroArchSeries(distroseries=distroseries)
script = self.makeScript(das.distroseries.distribution)
- self.assertEqual([das.architecturetag], script.getArchs())
+ self.assertEqual(
+ [das.architecturetag], script.getArchs(distroseries.name))
- def test_getSuites(self):
- # getSuites returns the suites in the distribution. The main
- # suite has the same name as the distro, without suffix.
+ def test_getSupportedSeries(self):
+ # getSupportedSeries returns the supported distroseries in the
+ # distribution.
script = self.makeScript()
distroseries = self.factory.makeDistroSeries(
distribution=script.distribution)
- self.assertIn(distroseries.name, script.getSuites())
+ self.assertIn(distroseries, script.getSupportedSeries())
- def test_getPockets(self):
- # getPockets returns the full names (distroseries-pocket) of the
+ def test_getSuites(self):
+ # getSuites returns the full names (distroseries-pocket) of the
# pockets that have packages to publish.
distro = self.makeDistro()
distroseries = self.factory.makeDistroSeries(distribution=distro)
package = self.factory.makeSuiteSourcePackage(distroseries)
script = self.makeScript(distro)
os.makedirs(os.path.join(script.config.distsroot, package.suite))
- self.assertEqual([package.suite], script.getPockets())
+ self.assertEqual([package.suite], list(script.getSuites()))
- def test_getPockets_includes_release_pocket(self):
- # getPockets also includes the release pocket, which is named
+ def test_getSuites_includes_release_pocket(self):
+ # getSuites also includes the release pocket, which is named
# after the distroseries without a suffix.
distro = self.makeDistro()
distroseries = self.factory.makeDistroSeries(distribution=distro)
@@ -244,7 +237,7 @@
distroseries, pocket=PackagePublishingPocket.RELEASE)
script = self.makeScript(distro)
os.makedirs(os.path.join(script.config.distsroot, package.suite))
- self.assertEqual([package.suite], script.getPockets())
+ self.assertEqual([package.suite], list(script.getSuites()))
def test_writeAptContentsConf_writes_header(self):
# writeAptContentsConf writes apt-contents.conf. At a minimum
@@ -252,7 +245,7 @@
# with the right distribution name interpolated.
distro = self.makeDistro()
script = self.makeScript(distro)
- script.writeAptContentsConf([], [])
+ script.writeAptContentsConf([])
apt_contents_conf = file(
"%s/%s-misc/apt-contents.conf"
% (script.content_archive, distro.name)).read()
@@ -264,19 +257,22 @@
# apt_conf_dist.template for every suite, with certain
# parameters interpolated.
distro = self.makeDistro()
+ distroseries = self.factory.makeDistroSeries(distro)
+ das = self.factory.makeDistroArchSeries(distroseries=distroseries)
script = self.makeScript(distro)
content_archive = script.content_archive
- suite = self.factory.getUniqueString('suite')
- arch = self.factory.getUniqueString('arch')
- script.writeAptContentsConf([suite], [arch])
+ script.writeAptContentsConf([distroseries.name])
apt_contents_conf = file(
"%s/%s-misc/apt-contents.conf"
% (script.content_archive, distro.name)).read()
- self.assertIn('tree "dists/%s"\n' % suite, apt_contents_conf)
+ self.assertIn(
+ 'tree "dists/%s"\n' % distroseries.name, apt_contents_conf)
overrides_path = os.path.join(
content_archive, distro.name + "-overrides")
self.assertIn('FileList "%s' % overrides_path, apt_contents_conf)
- self.assertIn('Architectures "%s source";' % arch, apt_contents_conf)
+ self.assertIn(
+ 'Architectures "%s source";' % das.architecturetag,
+ apt_contents_conf)
def test_writeContentsTop(self):
# writeContentsTop writes a Contents.top file based on a
@@ -314,7 +310,7 @@
suite = package.suite
script = self.makeScript(distro)
os.makedirs(os.path.join(script.config.distsroot, package.suite))
- self.assertNotEqual([], script.getPockets())
+ self.assertNotEqual([], list(script.getSuites()))
fake_overrides(script, distroseries)
script.process()
self.assertTrue(file_exists(os.path.join(
=== modified file 'lib/lp/soyuz/scripts/querydistro.py'
--- lib/lp/soyuz/scripts/querydistro.py 2012-04-16 13:56:45 +0000
+++ lib/lp/soyuz/scripts/querydistro.py 2012-06-20 01:32:25 +0000
@@ -26,7 +26,7 @@
Also initialize the list 'allowed_arguments'.
"""
- self.allowed_actions = ['development', 'supported', 'archs']
+ self.allowed_actions = ['supported']
self.usage = '%%prog <%s>' % ' | '.join(self.allowed_actions)
LaunchpadScript.__init__(self, *args, **kwargs)
@@ -118,35 +118,6 @@
"Action does not accept defined suite.")
@property
- def development(self):
- """Return the name of the DEVELOPMENT distroseries.
-
- It is restricted for the context distribution.
-
- It may raise `LaunchpadScriptFailure` if a suite was passed on the
- command-line.
-
- Return the first FROZEN distroseries found if there is no
- DEVELOPMENT one available.
-
- Raises `NotFoundError` if neither a CURRENT nor a FROZEN
- candidate could be found.
- """
- self.checkNoSuiteDefined()
- series = None
- wanted_status = (SeriesStatus.DEVELOPMENT,
- SeriesStatus.FROZEN)
- for status in wanted_status:
- series = self.location.distribution.getSeriesByStatus(status)
- if series.count() > 0:
- break
- else:
- raise LaunchpadScriptFailure(
- 'There is no DEVELOPMENT distroseries for %s' %
- self.location.distribution.name)
- return series[0].name
-
- @property
def supported(self):
"""Return the names of the distroseries currently supported.
@@ -174,12 +145,3 @@
self.location.distribution.name)
return " ".join(supported_series)
-
- @property
- def archs(self):
- """Return a space-separated list of architecture tags.
-
- It is restricted for the context distribution and suite.
- """
- architectures = self.location.distroseries.architectures
- return " ".join(arch.architecturetag for arch in architectures)
=== modified file 'lib/lp/soyuz/scripts/tests/test_lpquerydistro.py'
--- lib/lp/soyuz/scripts/tests/test_lpquerydistro.py 2012-04-16 13:56:45 +0000
+++ lib/lp/soyuz/scripts/tests/test_lpquerydistro.py 2012-06-20 01:32:25 +0000
@@ -12,9 +12,7 @@
from zope.component import getUtility
from lp.registry.interfaces.distribution import IDistributionSet
-from lp.registry.interfaces.series import SeriesStatus
from lp.services.config import config
-from lp.services.database.sqlbase import flush_database_updates
from lp.services.scripts.base import LaunchpadScriptFailure
from lp.soyuz.scripts.querydistro import LpQueryDistro
from lp.testing.layers import (
@@ -50,15 +48,15 @@
Check that:
* return code is ZERO,
* standard error is empty
- * standard output contains only the 'development distroseries' name
+ * standard output contains only the 'supported distroseries' names
"""
returncode, out, err = self.runLpQueryDistro(
- extra_args=['development'])
+ extra_args=['supported'])
self.assertEqual(
0, returncode, "\nScript Failed:%s\nStdout:\n%s\nStderr\n%s\n"
% (returncode, out, err))
- self.assertEqual(out.strip(), 'hoary')
+ self.assertEqual(out.strip(), 'hoary warty')
self.assertEqual(err.strip(), '')
def testMissingAction(self):
@@ -108,7 +106,7 @@
* standard error contains additional information about the failure.
"""
returncode, out, err = self.runLpQueryDistro(
- extra_args=['-s', 'warty', 'development'])
+ extra_args=['-s', 'warty', 'supported'])
self.assertEqual(
1, returncode,
@@ -140,24 +138,6 @@
"""
self.test_output = '%s' % args
- def testDevelopmentAndFrozenDistroSeries(self):
- """The 'development' action should cope with FROZEN distroseries."""
- helper = self.getLpQueryDistro(test_args=['development'])
- helper.runAction(presenter=self.presenter)
- hoary = self.ubuntu['hoary']
- self.assertEqual(hoary.status.name, 'DEVELOPMENT')
- self.assertEqual(helper.location.distribution.name, u'ubuntu')
- self.assertEqual(self.test_output, u'hoary')
-
- hoary.status = SeriesStatus.FROZEN
- flush_database_updates()
-
- helper = self.getLpQueryDistro(test_args=['development'])
- helper.runAction(presenter=self.presenter)
- self.assertEqual(hoary.status.name, 'FROZEN')
- self.assertEqual(helper.location.distribution.name, u'ubuntu')
- self.assertEqual(self.test_output, u'hoary')
-
def testUnknownAction(self):
"""'runAction' should fail for an unknown action."""
helper = self.getLpQueryDistro(test_args=['boom'])
@@ -177,7 +157,7 @@
See testActionswithDefinedSuite for further information.
"""
helper = self.getLpQueryDistro(
- test_args=['-s', 'warty', 'development'])
+ test_args=['-s', 'warty', 'supported'])
self.assertRaises(LaunchpadScriptFailure,
helper.runAction, self.presenter)
@@ -205,16 +185,12 @@
def testActionsWithUndefinedSuite(self):
"""Check the actions supposed to work with undefined suite.
- Only 'development' and 'supported' work with undefined suite.
- The other actions ('archs') will assume the CURRENT distroseries in
- context.
+ Only 'supported' works with undefined suite.
"""
helper = self.getLpQueryDistro(test_args=[])
helper._buildLocation()
- self.assertEqual(helper.development, 'hoary')
self.assertEqual(helper.supported, 'hoary warty')
- self.assertEqual(helper.archs, 'hppa i386')
def assertAttributeRaisesScriptFailure(self, obj, attr_name):
"""Asserts if accessing the given attribute name fails.
@@ -227,13 +203,10 @@
def testActionsWithDefinedSuite(self):
"""Opposite of testActionsWithUndefinedSuite.
- Only some actions ('archs') work with defined suite, the other
- actions ('development' and 'supported') will raise
- LaunchpadScriptError if the suite is defined.
+ The 'supported' action will raise LaunchpadScriptError if the suite
+ is defined.
"""
helper = self.getLpQueryDistro(test_args=['-s', 'warty'])
helper._buildLocation()
- self.assertAttributeRaisesScriptFailure(helper, 'development')
self.assertAttributeRaisesScriptFailure(helper, 'supported')
- self.assertEqual(helper.archs, 'hppa i386')
=== modified file 'scripts/ftpmaster-tools/lp-query-distro.py'
--- scripts/ftpmaster-tools/lp-query-distro.py 2012-04-16 13:56:45 +0000
+++ scripts/ftpmaster-tools/lp-query-distro.py 2012-06-20 01:32:25 +0000
@@ -9,10 +9,8 @@
It should provide an easy way to retrieve current information from
Launchpad when using plain shell scripts, for example:
- * DEVELOPMENT distroseries name:
- `./ubuntu-helper.py -d ubuntu development`
- * Distroseries architectures:
- `./lp-query-distro.py -d ubuntu -s feisty archs`
+ * SUPPORTED distroseries names:
+ `./lp-query-distro.py -d ubuntu supported`
Standard Output will carry the successfully executed information and
exit_code will be ZERO.
Follow ups
