launchpad-reviewers team mailing list archive

[Colin Watson <mp+415084@xxxxxxxxxxxxxxxxxx>]

Colin Watson has proposed merging ~cjwatson/launchpad:avoid-showing-duplicate-builder into launchpad:master.

Commit message:
Avoid showing duplicate builder information on build pages

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/+git/launchpad/+merge/415084

Build pages previously assumed (perhaps not very intentionally) that `BuildFarmJob.builder` wasn't set until the build finished.  However, following https://code.launchpad.net/~cjwatson/launchpad/+git/launchpad/+merge/414070, we now set `BuildFarmJob.builder` as soon as we handle any status response from the builder running the build, which isn't quite as early as it could be but it's pretty close.  This means that we've started rendering the builder name twice for builds in the `BUILDING` state, which is unsightly.

Simplify build pages so that they only consider `BuildFarmJob.builder` regardless of the build state, which avoids sometimes rendering the builder name twice.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~cjwatson/launchpad:avoid-showing-duplicate-builder into launchpad:master.

diff --git a/lib/lp/buildmaster/model/buildfarmjobbehaviour.py b/lib/lp/buildmaster/model/buildfarmjobbehaviour.py
index 1792d78..dc49901 100644
--- a/lib/lp/buildmaster/model/buildfarmjobbehaviour.py
+++ b/lib/lp/buildmaster/model/buildfarmjobbehaviour.py
@@ -327,9 +327,6 @@ class BuildFarmJobBehaviourBase:
         # atomically.  Setting the status to UPLOADING constitutes handoff to
         # process-upload, so doing that before we've removed the BuildQueue
         # causes races.
-
-        # XXX wgrant: The builder should be set long before here, but
-        # currently isn't.
         self.build.updateStatus(
             build_status, builder=bq.builder, worker_status=worker_status)
 
diff --git a/lib/lp/charms/templates/charmrecipebuild-index.pt b/lib/lp/charms/templates/charmrecipebuild-index.pt
index 95115bf..86170d6 100644
--- a/lib/lp/charms/templates/charmrecipebuild-index.pt
+++ b/lib/lp/charms/templates/charmrecipebuild-index.pt
@@ -88,14 +88,10 @@
       <span tal:attributes="
             class string:buildstatus${context/status/name};"
             tal:content="context/status/title"/>
-      <tal:building condition="context/status/enumvalue:BUILDING">
-        on <a tal:content="context/buildqueue_record/builder/title"
-              tal:attributes="href context/buildqueue_record/builder/fmt:url"/>
-      </tal:building>
-      <tal:built condition="context/builder">
+      <tal:builder condition="context/builder">
         on <a tal:content="context/builder/title"
               tal:attributes="href context/builder/fmt:url"/>
-      </tal:built>
+      </tal:builder>
       <tal:retry define="link context/menu:context/retry"
                  condition="link/enabled"
                  replace="structure link/fmt:link" />
diff --git a/lib/lp/code/templates/sourcepackagerecipebuild-index.pt b/lib/lp/code/templates/sourcepackagerecipebuild-index.pt
index 7f9b011..dcf7018 100644
--- a/lib/lp/code/templates/sourcepackagerecipebuild-index.pt
+++ b/lib/lp/code/templates/sourcepackagerecipebuild-index.pt
@@ -94,14 +94,10 @@
       <span tal:attributes="
             class string:buildstatus${context/status/name};"
             tal:content="context/status/title">Fully built</span>
-      <tal:building condition="context/status/enumvalue:BUILDING">
-        on <a tal:content="context/buildqueue_record/builder/title"
-              tal:attributes="href context/buildqueue_record/builder/fmt:url"/>
-      </tal:building>
-      <tal:built condition="context/builder">
+      <tal:builder condition="context/builder">
         on <a tal:content="context/builder/title"
               tal:attributes="href context/builder/fmt:url"/>
-      </tal:built>
+      </tal:builder>
     </p>
 
     <ul>
diff --git a/lib/lp/oci/templates/ocirecipebuild-index.pt b/lib/lp/oci/templates/ocirecipebuild-index.pt
index a1f3caf..d0b49c2 100644
--- a/lib/lp/oci/templates/ocirecipebuild-index.pt
+++ b/lib/lp/oci/templates/ocirecipebuild-index.pt
@@ -75,14 +75,10 @@
       <span tal:attributes="
             class string:buildstatus${context/status/name};"
             tal:content="context/status/title"/>
-      <tal:building condition="context/status/enumvalue:BUILDING">
-        on <a tal:content="context/buildqueue_record/builder/title"
-              tal:attributes="href context/buildqueue_record/builder/fmt:url"/>
-      </tal:building>
-      <tal:built condition="context/builder">
+      <tal:builder condition="context/builder">
         on <a tal:content="context/builder/title"
               tal:attributes="href context/builder/fmt:url"/>
-      </tal:built>
+      </tal:builder>
       <tal:retry define="link context/menu:context/retry"
                  condition="link/enabled"
                  replace="structure link/fmt:link" />
diff --git a/lib/lp/snappy/templates/snapbuild-index.pt b/lib/lp/snappy/templates/snapbuild-index.pt
index c8d5daf..ea087c4 100644
--- a/lib/lp/snappy/templates/snapbuild-index.pt
+++ b/lib/lp/snappy/templates/snapbuild-index.pt
@@ -98,14 +98,10 @@
       <span tal:attributes="
             class string:buildstatus${context/status/name};"
             tal:content="context/status/title"/>
-      <tal:building condition="context/status/enumvalue:BUILDING">
-        on <a tal:content="context/buildqueue_record/builder/title"
-              tal:attributes="href context/buildqueue_record/builder/fmt:url"/>
-      </tal:building>
-      <tal:built condition="context/builder">
+      <tal:builder condition="context/builder">
         on <a tal:content="context/builder/title"
               tal:attributes="href context/builder/fmt:url"/>
-      </tal:built>
+      </tal:builder>
       <tal:retry define="link context/menu:context/retry"
                  condition="link/enabled"
                  replace="structure link/fmt:link" />
diff --git a/lib/lp/soyuz/browser/build.py b/lib/lp/soyuz/browser/build.py
index 2ff61ac..be9c034 100644
--- a/lib/lp/soyuz/browser/build.py
+++ b/lib/lp/soyuz/browser/build.py
@@ -249,10 +249,6 @@ class BuildView(LaunchpadView):
         return self.context.archive.is_ppa
 
     @cachedproperty
-    def buildqueue(self):
-        return self.context.buildqueue_record
-
-    @cachedproperty
     def component_name(self):
         # Production has some buggy historic builds without
         # source publications.
diff --git a/lib/lp/soyuz/browser/tests/test_build_views.py b/lib/lp/soyuz/browser/tests/test_build_views.py
index 477a9f6..2f734d1 100644
--- a/lib/lp/soyuz/browser/tests/test_build_views.py
+++ b/lib/lp/soyuz/browser/tests/test_build_views.py
@@ -120,7 +120,7 @@ class TestBuildViews(TestCaseWithFactory):
         build_view = getMultiAdapter(
             (build, self.empty_request), name="+index")
         self.assertFalse(build_view.is_ppa)
-        self.assertEqual(build_view.buildqueue, None)
+        self.assertEqual(build.buildqueue_record, None)
         self.assertEqual(build_view.component_name, 'multiverse')
         self.assertFalse(build.can_be_retried)
         self.assertBuildMenuRetryIsExpected(build, build.archive.owner, False)
diff --git a/lib/lp/soyuz/stories/soyuz/xx-build-record.txt b/lib/lp/soyuz/stories/soyuz/xx-build-record.txt
index 33bb61b..56ab960 100644
--- a/lib/lp/soyuz/stories/soyuz/xx-build-record.txt
+++ b/lib/lp/soyuz/stories/soyuz/xx-build-record.txt
@@ -160,7 +160,7 @@ messages.
     >>> import pytz
     >>> now = datetime.datetime.now(pytz.UTC)
     >>> build.updateStatus(
-    ...     BuildStatus.BUILDING,
+    ...     BuildStatus.BUILDING, builder=bob_builder,
     ...     date_started=(now - datetime.timedelta(minutes=1)))
     >>> build.buildqueue_record.markAsBuilding(bob_builder)
     >>> build.buildqueue_record.logtail = 'one line\nanother line'
diff --git a/lib/lp/soyuz/templates/build-index.pt b/lib/lp/soyuz/templates/build-index.pt
index 1ae197e..4910e03 100644
--- a/lib/lp/soyuz/templates/build-index.pt
+++ b/lib/lp/soyuz/templates/build-index.pt
@@ -125,14 +125,10 @@
       <span tal:attributes="
             class string:buildstatus${context/status/name};"
             tal:content="context/status/title">Fully built</span>
-      <tal:building condition="context/status/enumvalue:BUILDING">
-        on <a tal:content="view/buildqueue/builder/title"
-              tal:attributes="href view/buildqueue/builder/fmt:url"/>
-      </tal:building>
-      <tal:built condition="context/builder">
+      <tal:builder condition="context/builder">
         on <a tal:content="context/builder/title"
               tal:attributes="href context/builder/fmt:url"/>
-      </tal:built>
+      </tal:builder>
       <tal:retry define="link context/menu:context/retry"
                  condition="link/enabled"
                  replace="structure link/fmt:link" />
@@ -152,8 +148,8 @@
             replace="eta/fmt:approximatedate">in 3 hours</tal:eta>
         </li>
       </tal:pending>
-      <li tal:condition="view/buildqueue">
-        Build score:<span tal:replace="view/buildqueue/lastscore"/>
+      <li tal:condition="context/buildqueue_record">
+        Build score:<span tal:replace="context/buildqueue_record/lastscore"/>
           <tal:rescore define="link context/menu:context/rescore"
                        condition="link/enabled"
                        replace="structure link/fmt:icon" />
@@ -163,8 +159,8 @@
       <tal:building condition="context/status/enumvalue:BUILDING">
         <li>
           Started
-          <span tal:attributes="title view/buildqueue/date_started/fmt:datetime"
-                tal:content="view/buildqueue/date_started/fmt:approximatedate"
+          <span tal:attributes="title context/buildqueue_record/date_started/fmt:datetime"
+                tal:content="context/buildqueue_record/date_started/fmt:approximatedate"
             >5 minutes ago</span>
         </li>
       </tal:building>
diff --git a/lib/lp/soyuz/templates/livefsbuild-index.pt b/lib/lp/soyuz/templates/livefsbuild-index.pt
index dd715d3..cff5cb2 100644
--- a/lib/lp/soyuz/templates/livefsbuild-index.pt
+++ b/lib/lp/soyuz/templates/livefsbuild-index.pt
@@ -110,14 +110,10 @@
       <span tal:attributes="
             class string:buildstatus${context/status/name};"
             tal:content="context/status/title"/>
-      <tal:building condition="context/status/enumvalue:BUILDING">
-        on <a tal:content="context/buildqueue_record/builder/title"
-              tal:attributes="href context/buildqueue_record/builder/fmt:url"/>
-      </tal:building>
-      <tal:built condition="context/builder">
+      <tal:builder condition="context/builder">
         on <a tal:content="context/builder/title"
               tal:attributes="href context/builder/fmt:url"/>
-      </tal:built>
+      </tal:builder>
       <tal:cancel define="link context/menu:context/cancel"
                   condition="link/enabled"
                   replace="structure link/fmt:link" />