launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #02290
[Merge] lp:~thumper/launchpad/fix-recipe-build-oops into lp:launchpad
Tim Penhey has proposed merging lp:~thumper/launchpad/fix-recipe-build-oops into lp:launchpad.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
Related bugs:
#692814 Sourcepacakgerecipebuild tales formatting fails with deleted recipes
https://bugs.launchpad.net/bugs/692814
For more details, see:
https://code.launchpad.net/~thumper/launchpad/fix-recipe-build-oops/+merge/45319
This change updates the canonical_url for a source package recipe build. Prior to this change
the build lived under the recipe itself. However to maintain some traceability of builds
we now allow the builds to exist for deleted recipes. This made them very hard to traverse
to. The solution is to move the builds under the archive, just like the binary package builds.
However to have them co-exist, I needed to change the integral identifier at the end of the +build
to be the build farm job id, rather than the build id itself.
--
https://code.launchpad.net/~thumper/launchpad/fix-recipe-build-oops/+merge/45319
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~thumper/launchpad/fix-recipe-build-oops into lp:launchpad.
=== modified file 'lib/canonical/launchpad/doc/canonical_url_examples.txt'
--- lib/canonical/launchpad/doc/canonical_url_examples.txt 2010-10-19 18:44:31 +0000
+++ lib/canonical/launchpad/doc/canonical_url_examples.txt 2011-01-06 07:21:24 +0000
@@ -391,4 +391,4 @@
>>> build = factory.makeSourcePackageRecipeBuild(recipe=recipe)
>>> canonical_url(build)
- u'http://code.launchpad.dev/~person-name.../+recipe/generic-string.../+build/...'
+ u'http://code.launchpad.dev/~person-name.../+archive/generic-string.../+build/...'
=== modified file 'lib/lp/app/browser/tales.py'
--- lib/lp/app/browser/tales.py 2010-11-03 08:17:36 +0000
+++ lib/lp/app/browser/tales.py 2011-01-06 07:21:24 +0000
@@ -1645,10 +1645,15 @@
class SourcePackageRecipeBuildFormatterAPI(CustomizableFormatter):
"""Adapter providing fmt support for ISourcePackageRecipe objects."""
- _link_summary_template = '%(name)s recipe build [%(owner)s/%(archive)s]'
+ _link_summary_template = '%(name)s [%(owner)s/%(archive)s]'
def _link_summary_values(self):
- return {'name': self._context.recipe.base_branch.unique_name,
+ if self._context.recipe is None:
+ name = 'build for deleted recipe'
+ else:
+ branch_name = self._context.recipe.base_branch.unique_name
+ name = '%s recipe build' % branch_name
+ return {'name': name,
'owner': self._context.archive.owner.name,
'archive': self._context.archive.name}
=== modified file 'lib/lp/buildmaster/interfaces/packagebuild.py'
--- lib/lp/buildmaster/interfaces/packagebuild.py 2010-11-16 10:55:23 +0000
+++ lib/lp/buildmaster/interfaces/packagebuild.py 2011-01-06 07:21:24 +0000
@@ -37,6 +37,10 @@
id = Attribute('The package build ID.')
+ url_id = Attribute(
+ 'A unique identifier for accessing the builds. '
+ 'Used for the canonical_url generation.')
+
archive = exported(
Reference(
title=_('Archive'), schema=IArchive,
=== modified file 'lib/lp/buildmaster/model/packagebuild.py'
--- lib/lp/buildmaster/model/packagebuild.py 2010-12-15 22:30:35 +0000
+++ lib/lp/buildmaster/model/packagebuild.py 2011-01-06 07:21:24 +0000
@@ -94,6 +94,10 @@
build_farm_job_id = Int(name='build_farm_job', allow_none=False)
build_farm_job = Reference(build_farm_job_id, 'BuildFarmJob.id')
+ @property
+ def url_id(self):
+ return self.build_farm_job_id
+
# The following two properties are part of the IPackageBuild
# interface, but need to be provided by derived classes.
distribution = None
=== modified file 'lib/lp/code/browser/configure.zcml'
--- lib/lp/code/browser/configure.zcml 2010-12-15 06:47:38 +0000
+++ lib/lp/code/browser/configure.zcml 2011-01-06 07:21:24 +0000
@@ -1195,8 +1195,8 @@
rootsite="code" />
<browser:url
for="lp.code.interfaces.sourcepackagerecipebuild.ISourcePackageRecipeBuild"
- attribute_to_parent="recipe"
- path_expression="string:+build/${id}"
+ attribute_to_parent="archive"
+ path_expression="string:+build/${url_id}"
rootsite="code" />
<browser:menus
classes="SourcePackageRecipeBuildContextMenu"
=== modified file 'lib/lp/code/browser/tests/test_sourcepackagerecipe.py'
--- lib/lp/code/browser/tests/test_sourcepackagerecipe.py 2010-12-22 02:01:36 +0000
+++ lib/lp/code/browser/tests/test_sourcepackagerecipe.py 2011-01-06 07:21:24 +0000
@@ -1192,8 +1192,7 @@
"""Test the basic index page."""
main_text = self.getMainText(self.makeBuild(), '+index')
self.assertTextMatchesExpressionIgnoreWhitespace("""\
- Code
- my-recipe
+ Owner Code PPA named build for Owner
created .*
Build status
Needs building
@@ -1223,8 +1222,7 @@
main_text = self.getMainText(
release.source_package_recipe_build, '+index')
self.assertTextMatchesExpressionIgnoreWhitespace("""\
- Code
- my-recipe
+ Owner Code PPA named build for Owner
created .*
Build status
Successfully built
=== modified file 'lib/lp/code/browser/tests/test_tales.py'
--- lib/lp/code/browser/tests/test_tales.py 2010-10-04 19:50:45 +0000
+++ lib/lp/code/browser/tests/test_tales.py 2011-01-06 07:21:24 +0000
@@ -9,12 +9,14 @@
import unittest
from storm.store import Store
+from testtools.matchers import Equals
from zope.security.proxy import removeSecurityProxy
from canonical.launchpad.webapp.publisher import canonical_url
from canonical.testing.layers import LaunchpadFunctionalLayer
from lp.testing import (
login,
+ person_logged_in,
test_tales,
TestCaseWithFactory,
)
@@ -174,6 +176,38 @@
diff.diff_text.getURL(), test_tales('diff/fmt:url', diff=diff))
+class TestSourcePackageRecipeBuild(TestCaseWithFactory):
+ """Test the formatter for SourcePackageRecipeBuilds."""
+
+ layer = LaunchpadFunctionalLayer
+
+ def test_link(self):
+ eric = self.factory.makePerson(name='eric')
+ ppa = self.factory.makeArchive(owner=eric, name='ppa')
+ build = self.factory.makeSourcePackageRecipeBuild(
+ archive=ppa)
+ self.assertThat(
+ test_tales('build/fmt:link', build=build),
+ Equals(
+ '<a href="http://code.launchpad.dev/~eric/+archive/ppa/'
+ '+build/%s">%s recipe build [eric/ppa]</a>'
+ % (build.url_id, build.recipe.base_branch.unique_name)))
+
+ def test_link_no_recipe(self):
+ eric = self.factory.makePerson(name='eric')
+ ppa = self.factory.makeArchive(owner=eric, name='ppa')
+ build = self.factory.makeSourcePackageRecipeBuild(
+ archive=ppa)
+ with person_logged_in(build.recipe.owner):
+ build.recipe.destroySelf()
+ self.assertThat(
+ test_tales('build/fmt:link', build=build),
+ Equals(
+ '<a href="http://code.launchpad.dev/~eric/+archive/ppa/'
+ '+build/%s">build for deleted recipe [eric/ppa]</a>'
+ % (build.url_id, )))
+
+
def test_suite():
return unittest.TestLoader().loadTestsFromName(__name__)
=== modified file 'lib/lp/code/mail/tests/test_sourcepackagerecipebuild.py'
--- lib/lp/code/mail/tests/test_sourcepackagerecipebuild.py 2010-12-08 17:22:23 +0000
+++ lib/lp/code/mail/tests/test_sourcepackagerecipebuild.py 2011-01-06 07:21:24 +0000
@@ -12,6 +12,7 @@
from zope.security.proxy import removeSecurityProxy
from canonical.config import config
+from canonical.launchpad.webapp import canonical_url
from canonical.testing.layers import LaunchpadFunctionalLayer
from lp.buildmaster.enums import BuildStatus
from lp.code.mail.sourcepackagerecipebuild import (
@@ -73,9 +74,9 @@
body, footer = ctrl.body.split('\n-- \n')
self.assertEqual(
expected_body % build.log.getURL(), body)
+ build_url = canonical_url(build)
self.assertEqual(
- 'http://code.launchpad.dev/~person/+recipe/recipe/+build/1\n'
- 'You are the requester of the build.\n', footer)
+ '%s\nYou are the requester of the build.\n' % build_url, footer)
self.assertEqual(
config.canonical.noreply_from_address, ctrl.from_addr)
self.assertEqual(
@@ -104,9 +105,9 @@
'Build for superseded Source' % (build.id), ctrl.subject)
body, footer = ctrl.body.split('\n-- \n')
self.assertEqual(superseded_body, body)
+ build_url = canonical_url(build)
self.assertEqual(
- 'http://code.launchpad.dev/~person/+recipe/recipe/+build/1\n'
- 'You are the requester of the build.\n', footer)
+ '%s\nYou are the requester of the build.\n' % build_url, footer)
self.assertEqual(
config.canonical.noreply_from_address, ctrl.from_addr)
self.assertEqual(
=== modified file 'lib/lp/soyuz/browser/archive.py'
--- lib/lp/soyuz/browser/archive.py 2010-12-20 05:46:40 +0000
+++ lib/lp/soyuz/browser/archive.py 2011-01-06 07:21:24 +0000
@@ -100,6 +100,7 @@
from lp.app.browser.stringformatter import FormattersAPI
from lp.app.errors import NotFoundError
from lp.buildmaster.enums import BuildStatus
+from lp.buildmaster.interfaces.buildfarmjob import IBuildFarmJobSet
from lp.registry.interfaces.person import (
IPersonSet,
PersonVisibility,
@@ -137,10 +138,7 @@
)
from lp.soyuz.interfaces.archivepermission import IArchivePermissionSet
from lp.soyuz.interfaces.archivesubscriber import IArchiveSubscriberSet
-from lp.soyuz.interfaces.binarypackagebuild import (
- BuildSetStatus,
- IBinaryPackageBuildSet,
- )
+from lp.soyuz.interfaces.binarypackagebuild import BuildSetStatus
from lp.soyuz.interfaces.binarypackagename import IBinaryPackageNameSet
from lp.soyuz.interfaces.component import IComponentSet
from lp.soyuz.interfaces.packagecopyrequest import IPackageCopyRequestSet
@@ -232,7 +230,8 @@
except ValueError:
return None
try:
- return getUtility(IBinaryPackageBuildSet).getByBuildID(build_id)
+ build_job = getUtility(IBuildFarmJobSet).getByID(build_id)
+ return build_job.getSpecificJob()
except NotFoundError:
return None
=== modified file 'lib/lp/soyuz/browser/build.py'
--- lib/lp/soyuz/browser/build.py 2010-12-16 12:48:14 +0000
+++ lib/lp/soyuz/browser/build.py 2011-01-06 07:21:24 +0000
@@ -84,7 +84,7 @@
@property
def path(self):
- return u"+build/%d" % self.context.id
+ return u"+build/%d" % self.context.url_id
class BuildNavigation(GetitemNavigation, FileNavigationMixin):
=== modified file 'lib/lp/soyuz/browser/distributionsourcepackagerelease.py'
--- lib/lp/soyuz/browser/distributionsourcepackagerelease.py 2010-08-31 11:31:04 +0000
+++ lib/lp/soyuz/browser/distributionsourcepackagerelease.py 2011-01-06 07:21:24 +0000
@@ -24,9 +24,9 @@
from canonical.lazr.utils import smartquote
from lp.app.errors import NotFoundError
from lp.archivepublisher.debversion import Version
+from lp.buildmaster.interfaces.buildfarmjob import IBuildFarmJobSet
from lp.services.propertycache import cachedproperty
from lp.soyuz.enums import PackagePublishingStatus
-from lp.soyuz.interfaces.binarypackagebuild import IBinaryPackageBuildSet
from lp.soyuz.interfaces.distributionsourcepackagerelease import (
IDistributionSourcePackageRelease,
)
@@ -50,7 +50,8 @@
except ValueError:
return None
try:
- return getUtility(IBinaryPackageBuildSet).getByBuildID(build_id)
+ build_job = getUtility(IBuildFarmJobSet).getByID(build_id)
+ return build_job.getSpecificJob()
except NotFoundError:
return None
=== modified file 'lib/lp/soyuz/stories/soyuz/xx-build-record.txt'
--- lib/lp/soyuz/stories/soyuz/xx-build-record.txt 2010-12-16 12:48:14 +0000
+++ lib/lp/soyuz/stories/soyuz/xx-build-record.txt 2011-01-06 07:21:24 +0000
@@ -543,7 +543,7 @@
...
>>> print anon_browser.getLink('~cprov/product/mybranch recipe build').url
- http://code.launchpad.dev/~cprov/+recipe/myrecipe/+build/1
+ http://code.launchpad.dev/~cprov/+archive/ppa/+build/...
Finally, the 'Build files' section is identical for PPA builds.
