launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #03530
[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