launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #14899
[Merge] lp:~wgrant/launchpad/no-+buildjob-redirect into lp:launchpad
William Grant has proposed merging lp:~wgrant/launchpad/no-+buildjob-redirect into lp:launchpad.
Commit message:
Drop the +buildjob BPB/SPRB redirect. It's been in place for more than 3 times as long as we generated the URLs, and it's now almost unused.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~wgrant/launchpad/no-+buildjob-redirect/+merge/142066
This branch eliminates the +buildjob redirect. It relies on BuildFarmJob.id, which we are dropping as part of the BFJ+PB -> BPB/SPRB/TTB flattening.
For a few months from late 2010 to March/April 2011, BinaryPackageBuild and SourcePackageRecipeBuild URLs were +buildjob/{BuildFarmJob.id}. I changed them back to +build/{BPB.id} and +recipebuild/{SPRB.id} nearly 2 years ago, leaving a compat redirect, and now there's only a couple of hundred monthly non-bot requests for them -- most of them directly to PPA binary packages that expired long ago, so 404 anyway. I don't think we lose significant value by dropping the redirect now.
--
https://code.launchpad.net/~wgrant/launchpad/no-+buildjob-redirect/+merge/142066
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~wgrant/launchpad/no-+buildjob-redirect into lp:launchpad.
=== modified file 'lib/lp/buildmaster/interfaces/buildfarmjob.py'
--- lib/lp/buildmaster/interfaces/buildfarmjob.py 2013-01-07 06:29:12 +0000
+++ lib/lp/buildmaster/interfaces/buildfarmjob.py 2013-01-07 08:20:30 +0000
@@ -173,13 +173,6 @@
vocabulary=BuildFarmJobType,
description=_("The specific type of job."))
- def getSpecificJob():
- """Return the specific build job associated with this record.
-
- :raises InconsistentBuildFarmJobError: if a specific job could not be
- returned.
- """
-
class IBuildFarmJob(Interface):
"""Operations that jobs for the build farm must implement."""
@@ -283,13 +276,6 @@
default=0,
description=_("Number of consecutive failures for this job."))
- def getSpecificJob():
- """Return the specific build job associated with this record.
-
- :raises InconsistentBuildFarmJobError: if a specific job could not be
- returned.
- """
-
def makeJob():
"""Create the specific job relating this with an lp.services.job.
@@ -372,7 +358,3 @@
that should be included.
:return: a `ResultSet` representing the requested builds.
"""
-
- def getByID(job_id):
- """Look up a `IBuildFarmJob` record by id.
- """
=== modified file 'lib/lp/buildmaster/model/buildfarmjob.py'
--- lib/lp/buildmaster/model/buildfarmjob.py 2013-01-07 01:54:48 +0000
+++ lib/lp/buildmaster/model/buildfarmjob.py 2013-01-07 08:20:30 +0000
@@ -27,18 +27,13 @@
Storm,
)
from storm.store import Store
-from zope.component import (
- ComponentLookupError,
- getUtility,
- )
+from zope.component import getUtility
from zope.interface import (
classProvides,
implements,
)
-from zope.proxy import isProxy
from zope.security.proxy import removeSecurityProxy
-from lp.app.errors import NotFoundError
from lp.buildmaster.enums import (
BuildFarmJobType,
BuildStatus,
@@ -48,8 +43,6 @@
IBuildFarmJobOld,
IBuildFarmJobSet,
IBuildFarmJobSource,
- InconsistentBuildFarmJobError,
- ISpecificBuildFarmJobSource,
)
from lp.buildmaster.interfaces.buildqueue import IBuildQueueSet
from lp.services.database.enumcol import DBEnum
@@ -279,31 +272,6 @@
store.add(build_farm_job)
return build_farm_job
- def getSpecificJob(self):
- """See `IBuild`"""
- # Adapt ourselves based on our job type.
- try:
- source = getUtility(
- ISpecificBuildFarmJobSource, self.job_type.name)
- except ComponentLookupError:
- raise InconsistentBuildFarmJobError(
- "No source was found for the build farm job type %s." % (
- self.job_type.name))
-
- build = source.getByBuildFarmJob(self)
-
- if build is None:
- raise InconsistentBuildFarmJobError(
- "There is no related specific job for the build farm "
- "job with id %d." % self.id)
-
- # Just to be on the safe side, make sure the build is still
- # proxied before returning it.
- assert isProxy(build), (
- "Unproxied result returned from ISpecificBuildFarmJobSource.")
-
- return build
-
class BuildFarmJobMixin:
@@ -397,11 +365,3 @@
return IStore(BuildFarmJob).using(*origin).find(
BuildFarmJob, *clauses).order_by(
Desc(BuildFarmJob.date_finished), BuildFarmJob.id)
-
- def getByID(self, job_id):
- """See `IBuildfarmJobSet`."""
- job = IStore(BuildFarmJob).find(BuildFarmJob,
- BuildFarmJob.id == job_id).one()
- if job is None:
- raise NotFoundError(job_id)
- return job
=== modified file 'lib/lp/buildmaster/tests/test_buildfarmjob.py'
--- lib/lp/buildmaster/tests/test_buildfarmjob.py 2013-01-07 05:25:16 +0000
+++ lib/lp/buildmaster/tests/test_buildfarmjob.py 2013-01-07 08:20:30 +0000
@@ -16,7 +16,6 @@
from zope.security.interfaces import Unauthorized
from zope.security.proxy import removeSecurityProxy
-from lp.app.errors import NotFoundError
from lp.app.interfaces.launchpad import ILaunchpadCelebrities
from lp.buildmaster.enums import (
BuildFarmJobType,
@@ -26,7 +25,6 @@
IBuildFarmJob,
IBuildFarmJobSet,
IBuildFarmJobSource,
- InconsistentBuildFarmJobError,
)
from lp.buildmaster.model.buildfarmjob import BuildFarmJob
from lp.services.database.sqlbase import flush_database_updates
@@ -103,20 +101,6 @@
self.assertEqual(None, bfj.builder)
self.assertEqual(None, bfj.log)
- def test_getSpecificJob_none(self):
- # An exception is raised if there is no related specific job.
- self.assertRaises(
- InconsistentBuildFarmJobError, self.build_farm_job.getSpecificJob)
-
- def test_getSpecificJob_unimplemented_type(self):
- # An `IBuildFarmJob` with an unimplemented type results in an
- # exception.
- removeSecurityProxy(self.build_farm_job).job_type = (
- BuildFarmJobType.RECIPEBRANCHBUILD)
-
- self.assertRaises(
- InconsistentBuildFarmJobError, self.build_farm_job.getSpecificJob)
-
def test_date_created(self):
# date_created can be passed optionally when creating a
# bulid farm job to ensure we don't get identical timestamps
@@ -286,17 +270,3 @@
result = self.build_farm_job_set.getBuildsForBuilder(self.builder)
self.assertEqual([build_1, build_2], list(result))
-
- def test_getByID(self):
- # getByID returns a job by id.
- build_1 = self.makeBuildFarmJob(
- builder=self.builder,
- date_finished=datetime(2008, 10, 10, tzinfo=pytz.UTC))
- flush_database_updates()
- self.assertEquals(
- build_1, self.build_farm_job_set.getByID(build_1.id))
-
- def test_getByID_nonexistant(self):
- # getByID raises NotFoundError for unknown job ids.
- self.assertRaises(NotFoundError,
- self.build_farm_job_set.getByID, 423432432432)
=== modified file 'lib/lp/code/model/tests/test_sourcepackagerecipebuild.py'
--- lib/lp/code/model/tests/test_sourcepackagerecipebuild.py 2012-12-26 01:32:19 +0000
+++ lib/lp/code/model/tests/test_sourcepackagerecipebuild.py 2013-01-07 08:20:30 +0000
@@ -520,13 +520,6 @@
BuildStatus.SUPERSEDED,
build.status)
- def test_getSpecificJob(self):
- # getSpecificJob returns the SourcePackageRecipeBuild
- sprb = self.makeSourcePackageRecipeBuild()
- Store.of(sprb).flush()
- job = sprb.build_farm_job.getSpecificJob()
- self.assertEqual(sprb, job)
-
def test_getUploader(self):
# For ACL purposes the uploader is the build requester.
build = self.makeSourcePackageRecipeBuild()
=== modified file 'lib/lp/soyuz/browser/build.py'
--- lib/lp/soyuz/browser/build.py 2013-01-07 06:15:07 +0000
+++ lib/lp/soyuz/browser/build.py 2013-01-07 08:20:30 +0000
@@ -44,7 +44,6 @@
)
from lp.buildmaster.enums import BuildStatus
from lp.buildmaster.interfaces.buildfarmjob import (
- IBuildFarmJobSet,
InconsistentBuildFarmJobError,
ISpecificBuildFarmJobSource,
)
@@ -146,19 +145,6 @@
except NotFoundError:
return None
- @stepthrough('+buildjob')
- def traverse_buildjob(self, name):
- try:
- job_id = int(name)
- except ValueError:
- return None
- try:
- build_job = getUtility(IBuildFarmJobSet).getByID(job_id)
- return self.redirectSubTree(
- canonical_url(build_job.getSpecificJob()))
- except NotFoundError:
- return None
-
class BuildFacets(StandardLaunchpadFacets):
"""The links that will appear in the facet menu for an
=== modified file 'lib/lp/soyuz/browser/tests/test_builder.py'
--- lib/lp/soyuz/browser/tests/test_builder.py 2012-04-16 23:02:44 +0000
+++ lib/lp/soyuz/browser/tests/test_builder.py 2013-01-07 08:20:30 +0000
@@ -11,53 +11,19 @@
from lp.buildmaster.interfaces.builder import IBuilderSet
from lp.buildmaster.interfaces.buildqueue import IBuildQueueSet
from lp.services.job.model.job import Job
-from lp.services.webapp import canonical_url
from lp.soyuz.browser.tests.test_builder_views import BuildCreationMixin
from lp.testing import (
record_two_runs,
TestCaseWithFactory,
)
-from lp.testing.layers import (
- DatabaseFunctionalLayer,
- LaunchpadFunctionalLayer,
- )
+from lp.testing.layers import LaunchpadFunctionalLayer
from lp.testing.matchers import HasQueryCount
-from lp.testing.publication import test_traverse
from lp.testing.views import create_initialized_view
from lp.translations.interfaces.translationtemplatesbuildjob import (
ITranslationTemplatesBuildJobSource,
)
-class TestBuildersNavigation(TestCaseWithFactory):
-
- layer = DatabaseFunctionalLayer
-
- def test_buildjob_redirects_for_recipe_build(self):
- # /builders/+buildjob/<job id> redirects to the build page.
- build = self.factory.makeSourcePackageRecipeBuild()
- url = 'http://launchpad.dev/builders/+buildjob/%s' % (
- build.build_farm_job.id)
- context, view, request = test_traverse(url)
- view()
- self.assertEqual(301, request.response.getStatus())
- self.assertEqual(
- canonical_url(build),
- request.response.getHeader('location'))
-
- def test_buildjob_redirects_for_binary_build(self):
- # /builders/+buildjob/<job id> redirects to the build page.
- build = self.factory.makeBinaryPackageBuild()
- url = 'http://launchpad.dev/builders/+buildjob/%s' % (
- build.build_farm_job.id)
- context, view, request = test_traverse(url)
- view()
- self.assertEqual(301, request.response.getStatus())
- self.assertEqual(
- canonical_url(build),
- request.response.getHeader('location'))
-
-
def builders_homepage_render():
builders = getUtility(IBuilderSet)
create_initialized_view(builders, "+index").render()
=== modified file 'lib/lp/soyuz/model/binarypackagebuild.py'
--- lib/lp/soyuz/model/binarypackagebuild.py 2013-01-04 07:26:21 +0000
+++ lib/lp/soyuz/model/binarypackagebuild.py 2013-01-07 08:20:30 +0000
@@ -801,12 +801,6 @@
LibraryFileAlias.id == BinaryPackageFile.libraryfileID,
LibraryFileAlias.filename == filename).one()
- def getSpecificJob(self):
- """See `IBuildFarmJob`."""
- # If we are asked to adapt an object that is already a binary
- # package build, then don't hit the db.
- return self
-
def getUploader(self, changes):
"""See `IBinaryPackageBuild`."""
return changes.signer
=== modified file 'lib/lp/soyuz/tests/test_binarypackagebuild.py'
--- lib/lp/soyuz/tests/test_binarypackagebuild.py 2013-01-07 06:29:12 +0000
+++ lib/lp/soyuz/tests/test_binarypackagebuild.py 2013-01-07 08:20:30 +0000
@@ -141,38 +141,6 @@
self.build.build_farm_job.id),
self.build.log_url)
- def test_adapt_from_build_farm_job(self):
- # An `IBuildFarmJob` can be adapted to an IBinaryPackageBuild
- # if it has the correct job type.
- build_farm_job = self.build.build_farm_job
- store = Store.of(build_farm_job)
- store.flush()
-
- self.assertEqual(self.build, build_farm_job.getSpecificJob())
-
- def test_adapt_from_build_farm_job_prefetching(self):
- # The package_build is prefetched for efficiency.
- build_farm_job = self.build.build_farm_job
-
- # We clear the cache to avoid getting cached objects where
- # they would normally be queries.
- store = Store.of(build_farm_job)
- store.flush()
- store.invalidate()
-
- binary_package_build = build_farm_job.getSpecificJob()
-
- self.assertStatementCount(
- 0, getattr, binary_package_build, "package_build")
- self.assertStatementCount(
- 0, getattr, binary_package_build, "build_farm_job")
-
- def test_getSpecificJob_noop(self):
- # If getSpecificJob is called on the binary build it is a noop.
- store = Store.of(self.build)
- store.flush()
- self.assertStatementCount(0, self.build.getSpecificJob)
-
def test_getUploader(self):
# For ACL purposes the uploader is the changes file signer.
=== modified file 'lib/lp/translations/tests/test_translationtemplatesbuild.py'
--- lib/lp/translations/tests/test_translationtemplatesbuild.py 2012-01-20 15:42:44 +0000
+++ lib/lp/translations/tests/test_translationtemplatesbuild.py 2013-01-07 08:20:30 +0000
@@ -78,14 +78,6 @@
self.assertEqual(1, len(builds))
self.assertIsInstance(builds[0], TranslationTemplatesBuild)
- def test_getSpecificJob(self):
- source = getUtility(ITranslationTemplatesBuildSource)
- build_farm_job = self._makeBuildFarmJob()
- branch = self.factory.makeBranch()
- build = source.create(build_farm_job, branch)
-
- self.assertEqual(build, build_farm_job.getSpecificJob())
-
def test_findByBranch(self):
source = getUtility(ITranslationTemplatesBuildSource)
build_farm_job = self._makeBuildFarmJob()
