launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #25429
[Merge] ~pappacena/launchpad:oci-unify-request-builds into launchpad:master
Thiago F. Pappacena has proposed merging ~pappacena/launchpad:oci-unify-request-builds into launchpad:master.
Commit message:
Use IOCIRecipeRequestBuildsJobSource when requesting builds from the web interface
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~pappacena/launchpad/+git/launchpad/+merge/391645
--
Your team Launchpad code reviewers is requested to review the proposed merge of ~pappacena/launchpad:oci-unify-request-builds into launchpad:master.
diff --git a/lib/lp/oci/browser/ocirecipe.py b/lib/lp/oci/browser/ocirecipe.py
index 5e6a50f..08def83 100644
--- a/lib/lp/oci/browser/ocirecipe.py
+++ b/lib/lp/oci/browser/ocirecipe.py
@@ -60,7 +60,6 @@ from lp.oci.interfaces.ocirecipe import (
NoSuchOCIRecipe,
OCI_RECIPE_ALLOW_CREATE,
OCI_RECIPE_WEBHOOKS_FEATURE_FLAG,
- OCIRecipeBuildAlreadyPending,
OCIRecipeFeatureDisabled,
)
from lp.oci.interfaces.ocirecipebuild import IOCIRecipeBuildSet
@@ -70,7 +69,6 @@ from lp.oci.interfaces.ociregistrycredentials import (
user_can_edit_credentials_for_owner,
)
from lp.services.features import getFeatureFlag
-from lp.services.helpers import english_list
from lp.services.propertycache import cachedproperty
from lp.services.webapp import (
canonical_url,
@@ -242,6 +240,10 @@ class OCIRecipeView(LaunchpadView):
else:
return "Built on request"
+ @property
+ def pending_build_requests(self):
+ return self.context.pending_build_requests.count()
+
def builds_for_recipe(recipe):
"""A list of interesting builds.
@@ -634,36 +636,12 @@ class OCIRecipeRequestBuildsView(LaunchpadFormView):
'distro_arch_series',
'You need to select at least one architecture.')
- def requestBuilds(self, data):
- """User action for requesting a number of builds.
-
- We raise exceptions for most errors, but if there's already a
- pending build for a particular architecture, we simply record that
- so that other builds can be queued and a message displayed to the
- caller.
- """
- informational = {}
- builds = []
- already_pending = []
- for arch in data['distro_arch_series']:
- try:
- build = self.context.requestBuild(self.user, arch)
- builds.append(build)
- except OCIRecipeBuildAlreadyPending:
- already_pending.append(arch)
- if already_pending:
- informational['already_pending'] = (
- "An identical build is already pending for %s." %
- english_list(arch.architecturetag for arch in already_pending))
- return builds, informational
-
@action('Request builds', name='request')
def request_action(self, action, data):
- builds, informational = self.requestBuilds(data)
- already_pending = informational.get('already_pending')
- notification_text = new_builds_notification_text(
- builds, already_pending)
- self.request.response.addNotification(notification_text)
+ self.context.requestBuilds(self.user, data['distro_arch_series'])
+ self.request.response.addNotification(
+ "Your builds were scheduled and should start soon. "
+ "Refresh this page for details.")
self.next_url = self.cancel_url
diff --git a/lib/lp/oci/browser/tests/test_ocirecipe.py b/lib/lp/oci/browser/tests/test_ocirecipe.py
index c854d14..4059e9f 100644
--- a/lib/lp/oci/browser/tests/test_ocirecipe.py
+++ b/lib/lp/oci/browser/tests/test_ocirecipe.py
@@ -50,18 +50,22 @@ from lp.oci.interfaces.ocirecipe import (
IOCIRecipeSet,
OCI_RECIPE_ALLOW_CREATE,
)
+from lp.oci.interfaces.ocirecipejob import IOCIRecipeRequestBuildsJobSource
from lp.oci.interfaces.ociregistrycredentials import (
IOCIRegistryCredentialsSet,
)
from lp.oci.tests.helpers import OCIConfigHelperMixin
from lp.registry.interfaces.person import IPersonSet
+from lp.services.config import config
from lp.services.database.constants import UTC_NOW
from lp.services.database.interfaces import IStore
from lp.services.features.testing import FeatureFixture
+from lp.services.job.runner import JobRunner
from lp.services.propertycache import get_property_cache
from lp.services.webapp import canonical_url
from lp.services.webapp.servers import LaunchpadTestRequest
from lp.testing import (
+ admin_logged_in,
anonymous_logged_in,
BrowserTestCase,
login,
@@ -71,6 +75,7 @@ from lp.testing import (
TestCaseWithFactory,
time_counter,
)
+from lp.testing.dbuser import dbuser
from lp.testing.layers import (
DatabaseFunctionalLayer,
LaunchpadFunctionalLayer,
@@ -836,6 +841,30 @@ class TestOCIRecipeRequestBuildsView(BaseTestOCIRecipeView):
self.assertRaises(
Unauthorized, self.getViewBrowser, self.recipe, "+request-builds")
+ def runRequestBuildJobs(self):
+ with admin_logged_in():
+ jobs = getUtility(IOCIRecipeRequestBuildsJobSource).iterReady()
+ with dbuser(config.IOCIRecipeRequestBuildsJobSource.dbuser):
+ JobRunner(jobs).runAll()
+
+ def test_pending_build_requests_not_shown_if_absent(self):
+ self.recipe.requestBuilds(self.recipe.owner)
+ browser = self.getViewBrowser(self.recipe, user=self.person)
+ content = extract_text(find_main_content(browser.contents))
+ self.assertIn(
+ "* You have 1 pending build request. "
+ "The builds should start automatically soon.",
+ content.replace("\n", " "))
+
+ # Run the build request, so we don't have any pending one. The
+ # message should be gone then.
+ self.runRequestBuildJobs()
+ browser = self.getViewBrowser(self.recipe, user=self.person)
+ content = extract_text(find_main_content(browser.contents))
+ self.assertNotIn(
+ "The builds should start automatically soon.",
+ content.replace("\n", " "))
+
def test_request_builds_action(self):
# Requesting a build creates pending builds.
browser = self.getViewBrowser(
@@ -844,6 +873,8 @@ class TestOCIRecipeRequestBuildsView(BaseTestOCIRecipeView):
self.assertTrue(browser.getControl("i386").selected)
browser.getControl("Request builds").click()
+ self.runRequestBuildJobs()
+
login_person(self.person)
builds = self.recipe.pending_builds
self.assertContentEqual(
@@ -852,19 +883,6 @@ class TestOCIRecipeRequestBuildsView(BaseTestOCIRecipeView):
self.assertContentEqual(
[2510], set(build.buildqueue_record.lastscore for build in builds))
- def test_request_builds_rejects_duplicate(self):
- # A duplicate build request causes a notification.
- self.recipe.requestBuild(self.person, self.distroseries["amd64"])
- browser = self.getViewBrowser(
- self.recipe, "+request-builds", user=self.person)
- self.assertTrue(browser.getControl("amd64").selected)
- self.assertTrue(browser.getControl("i386").selected)
- browser.getControl("Request builds").click()
- main_text = extract_text(find_main_content(browser.contents))
- self.assertIn("1 new build has been queued.", main_text)
- self.assertIn(
- "An identical build is already pending for amd64.", main_text)
-
def test_request_builds_no_architectures(self):
# Selecting no architectures causes a validation failure.
browser = self.getViewBrowser(
diff --git a/lib/lp/oci/interfaces/ocirecipe.py b/lib/lp/oci/interfaces/ocirecipe.py
index e185ea5..2b757e2 100644
--- a/lib/lp/oci/interfaces/ocirecipe.py
+++ b/lib/lp/oci/interfaces/ocirecipe.py
@@ -202,6 +202,9 @@ class IOCIRecipeView(Interface):
"The architectures that are available to be enabled or disabled for "
"this OCI recipe.")
+ pending_build_requests = Attribute(
+ "The list of build requests that didn't trigger builds yet.")
+
# This should only be set by using IOCIProject.setOfficialRecipe
official = Bool(
title=_("OCI project official"),
@@ -262,18 +265,10 @@ class IOCIRecipeView(Interface):
"Whether everything is set up to allow uploading builds of "
"this OCI recipe to a registry."))
- def requestBuild(requester, architecture):
- """Request that the OCI recipe is built.
-
- :param requester: The person requesting the build.
- :param architecture: The architecture to build for.
- :return: `IOCIRecipeBuild`.
- """
-
@call_with(requester=REQUEST_USER)
@export_factory_operation(IOCIRecipeBuildRequest, [])
@operation_for_version("devel")
- def requestBuilds(requester):
+ def requestBuilds(requester, distro_arch_series=None):
"""Request that the OCI recipe is built for all available
architectures.
diff --git a/lib/lp/oci/interfaces/ocirecipejob.py b/lib/lp/oci/interfaces/ocirecipejob.py
index 17d9fd5..c012151 100644
--- a/lib/lp/oci/interfaces/ocirecipejob.py
+++ b/lib/lp/oci/interfaces/ocirecipejob.py
@@ -81,13 +81,19 @@ class IOCIRecipeRequestBuildsJob(IRunnableJob):
class IOCIRecipeRequestBuildsJobSource(IJobSource):
- def create(oci_recipe, requester):
+ def create(oci_recipe, requester, distro_arch_series=None):
"""Request builds of an OCI Recipe.
:param oci_recipe: The OCI recipe to build.
:param requester: The person requesting the builds.
+ :param distro_arch_series: Build only for this list of
+ distro_arch_series, if they are available for the recipe. If
+ None, build for all available distro_arch_series.
"""
- def getByOCIRecipeAndID(self, oci_recipe, job_id):
+ def getByOCIRecipeAndID(oci_recipe, job_id):
"""Retrieve the build job by OCI recipe and the given job ID.
"""
+
+ def getPendingByOCIRecipe(oci_recipe, statuses):
+ """Retrieve the build job list by OCI recipe."""
diff --git a/lib/lp/oci/model/ocirecipe.py b/lib/lp/oci/model/ocirecipe.py
index ea03bf4..4be74b3 100644
--- a/lib/lp/oci/model/ocirecipe.py
+++ b/lib/lp/oci/model/ocirecipe.py
@@ -407,32 +407,40 @@ class OCIRecipe(Storm, WebhookTargetMixin):
def getBuildRequest(self, job_id):
return OCIRecipeBuildRequest(self, job_id)
- def requestBuildsFromJob(self, requester, build_request=None):
+ def requestBuildsFromJob(self, requester, build_request=None,
+ distro_arch_series=None):
self._checkRequestBuild(requester)
- distro_arch_series_to_build = set(self.getAllowedArchitectures())
+ if distro_arch_series is None:
+ distro_arch_series = set(self.getAllowedArchitectures())
builds = []
- for das in distro_arch_series_to_build:
+ for das in distro_arch_series:
try:
builds.append(self.requestBuild(
requester, das, build_request=build_request))
except OCIRecipeBuildAlreadyPending:
pass
- # If we have distro_arch_series_to_build, but they all failed to due
+ # If we have distro_arch_series, but they all failed to due
# to pending builds, we fail the job.
- if len(distro_arch_series_to_build) > 0 and len(builds) == 0:
+ if len(distro_arch_series) > 0 and len(builds) == 0:
raise OCIRecipeBuildAlreadyPending
return builds
- def requestBuilds(self, requester):
+ def requestBuilds(self, requester, distro_arch_series=None):
self._checkRequestBuild(requester)
job = getUtility(IOCIRecipeRequestBuildsJobSource).create(
- self, requester)
+ self, requester, distro_arch_series)
return self.getBuildRequest(job.job_id)
@property
+ def pending_build_requests(self):
+ util = getUtility(IOCIRecipeRequestBuildsJobSource)
+ return util.getPendingByOCIRecipe(
+ self, statuses=(JobStatus.WAITING, JobStatus.RUNNING))
+
+ @property
def push_rules(self):
rules = IStore(self).find(
OCIPushRule,
diff --git a/lib/lp/oci/model/ocirecipejob.py b/lib/lp/oci/model/ocirecipejob.py
index 095181e..9260cb4 100644
--- a/lib/lp/oci/model/ocirecipejob.py
+++ b/lib/lp/oci/model/ocirecipejob.py
@@ -162,9 +162,12 @@ class OCIRecipeRequestBuildsJob(OCIRecipeJobDerived):
config = config.IOCIRecipeRequestBuildsJobSource
@classmethod
- def create(cls, recipe, requester):
+ def create(cls, recipe, requester, distro_arch_series=None):
"""See `OCIRecipeRequestBuildsJob`."""
- metadata = {"requester": requester.id}
+ metadata = {
+ "requester": requester.id,
+ "distro_arch_series": [i.id for i in distro_arch_series or []]
+ }
recipe_job = OCIRecipeJob(recipe, cls.class_job_type, metadata)
job = cls(recipe_job)
job.celeryRunOnCommit()
@@ -180,6 +183,15 @@ class OCIRecipeRequestBuildsJob(OCIRecipeJobDerived):
raise NotFoundError("Could not find job ID %s" % job_id)
return cls(job)
+ @classmethod
+ def getPendingByOCIRecipe(cls, recipe, statuses):
+ return IStore(OCIRecipeJob).find(
+ (OCIRecipeJob, Job),
+ OCIRecipeJob.job_id == Job.id,
+ OCIRecipeJob.recipe == recipe,
+ OCIRecipeJob.job_type == cls.class_job_type,
+ Job._status.is_in(statuses))
+
def getOperationDescription(self):
return "requesting builds of %s" % self.recipe
@@ -234,6 +246,10 @@ class OCIRecipeRequestBuildsJob(OCIRecipeJobDerived):
"""See `OCIRecipeRequestBuildsJob`."""
self.metadata["builds"] = [build.id for build in builds]
+ @property
+ def distro_arch_series_ids(self):
+ return self.metadata.get("distro_arch_series") or []
+
def run(self):
"""See `IRunnableJob`."""
requester = self.requester
@@ -242,8 +258,13 @@ class OCIRecipeRequestBuildsJob(OCIRecipeJobDerived):
"Skipping %r because the requester has been deleted." % self)
return
try:
+ arch_series = set(self.recipe.getAllowedArchitectures())
+ if self.distro_arch_series_ids:
+ arch_series = {i for i in arch_series
+ if i.id in self.distro_arch_series_ids}
self.builds = self.recipe.requestBuildsFromJob(
- requester, build_request=self.build_request)
+ requester, build_request=self.build_request,
+ distro_arch_series=arch_series)
self.error_message = None
except self.retry_error_types:
raise
diff --git a/lib/lp/oci/templates/ocirecipe-index.pt b/lib/lp/oci/templates/ocirecipe-index.pt
index c2ef69e..0c042dd 100644
--- a/lib/lp/oci/templates/ocirecipe-index.pt
+++ b/lib/lp/oci/templates/ocirecipe-index.pt
@@ -72,6 +72,15 @@
</div>
<h2>Latest builds</h2>
+ <div tal:condition="view/pending_build_requests"
+ tal:define="count view/pending_build_requests|nothing;
+ plural string: build requests;
+ singular string: build request;">
+ * You have <span tal:replace="view/pending_build_requests" /> pending
+ <tal:plural
+ metal:use-macro="context/@@+base-layout-macros/plural-message"/>.
+ The builds should start automatically soon.
+ </div>
<table id="latest-builds-listing" class="listing"
style="margin-bottom: 1em;">
<thead>
diff --git a/lib/lp/oci/tests/test_ocirecipe.py b/lib/lp/oci/tests/test_ocirecipe.py
index 30c547f..94934c6 100644
--- a/lib/lp/oci/tests/test_ocirecipe.py
+++ b/lib/lp/oci/tests/test_ocirecipe.py
@@ -227,13 +227,13 @@ class TestOCIRecipe(OCIConfigHelperMixin, TestCaseWithFactory):
ocirecipe._hasPendingBuilds([arch_series_386, arch_series_hppa]))
def test_requestBuild(self):
- ocirecipe = self.factory.makeOCIRecipe()
+ ocirecipe = removeSecurityProxy(self.factory.makeOCIRecipe())
oci_arch = self.factory.makeOCIRecipeArch(recipe=ocirecipe)
build = ocirecipe.requestBuild(ocirecipe.owner, oci_arch)
self.assertEqual(build.status, BuildStatus.NEEDSBUILD)
def test_requestBuild_already_exists(self):
- ocirecipe = self.factory.makeOCIRecipe()
+ ocirecipe = removeSecurityProxy(self.factory.makeOCIRecipe())
oci_arch = self.factory.makeOCIRecipeArch(recipe=ocirecipe)
ocirecipe.requestBuild(ocirecipe.owner, oci_arch)
@@ -251,7 +251,8 @@ class TestOCIRecipe(OCIConfigHelperMixin, TestCaseWithFactory):
oci_arch = self.factory.makeOCIRecipeArch(recipe=recipe)
hook = self.factory.makeWebhook(
target=recipe, event_types=["oci-recipe:build:0.1"])
- build = recipe.requestBuild(recipe.owner, oci_arch)
+ build = removeSecurityProxy(recipe).requestBuild(
+ recipe.owner, oci_arch)
expected_payload = {
"recipe_build": Equals(
