launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #18193
Re: [Merge] lp:~cjwatson/launchpad/rearrange-builders into lp:launchpad
Review: Approve code
Diff comments:
> === modified file 'lib/lp/buildmaster/browser/builder.py'
> --- lib/lp/buildmaster/browser/builder.py 2014-06-20 11:52:32 +0000
> +++ lib/lp/buildmaster/browser/builder.py 2015-03-23 12:07:22 +0000
> @@ -1,4 +1,4 @@
> -# Copyright 2009-2014 Canonical Ltd. This software is licensed under the
> +# Copyright 2009-2015 Canonical Ltd. This software is licensed under the
> # GNU Affero General Public License version 3 (see the file LICENSE).
>
> """Browser views for builders."""
> @@ -16,6 +16,7 @@
> 'BuilderView',
> ]
>
> +from itertools import groupby
> import operator
>
> from lazr.restful.utils import smartquote
> @@ -37,8 +38,6 @@
> IBuilder,
> IBuilderSet,
> )
> -from lp.buildmaster.interfaces.buildqueue import IBuildQueueSet
> -from lp.buildmaster.model.buildqueue import BuildQueue
> from lp.code.interfaces.sourcepackagerecipebuild import (
> ISourcePackageRecipeBuildSource,
> )
> @@ -144,13 +143,26 @@
> def page_title(self):
> return self.label
>
> + @staticmethod
> + def getBuilderSortKey(builder):
> + return (
> + builder.virtualized,
> + tuple(p.id for p in builder.processors),
I'd explicitly sort this.
> + builder.name)
> +
> @cachedproperty
> def builders(self):
> """All active builders"""
> builders = list(self.context.getBuilders())
> - return list(sorted(
> - builders, key=lambda b: (
> - b.virtualized, tuple(p.id for p in b.processors), b.name)))
> + return list(sorted(builders, key=self.getBuilderSortKey))
> +
> + @property
> + def builder_clumps(self):
> + """Active builders grouped by virtualization and processors."""
> + return [
> + BuilderClump(list(group))
> + for _, group in groupby(
> + self.builders, lambda b: self.getBuilderSortKey(b)[:-1])]
>
> @property
> def number_of_registered_builders(self):
> @@ -176,21 +188,34 @@
>
> @property
> def virt_builders(self):
> - """Return a BuilderCategory object for PPA builders."""
> + """Return a BuilderCategory object for virtual builders."""
> builder_category = BuilderCategory(
> - 'PPA build status', virtualized=True)
> + 'Virtual build status', virtualized=True)
> builder_category.groupBuilders(self.builders, self.build_queue_sizes)
> return builder_category
>
> @property
> def nonvirt_builders(self):
> - """Return a BuilderCategory object for PPA builders."""
> + """Return a BuilderCategory object for non-virtual builders."""
> builder_category = BuilderCategory(
> - 'Official distributions build status', virtualized=False)
> + 'Non-virtual build status', virtualized=False)
> builder_category.groupBuilders(self.builders, self.build_queue_sizes)
> return builder_category
>
>
> +class BuilderClump:
> + """A "clump" of builders with the same virtualization and processors.
> +
> + The name came in desperation from a thesaurus; BuilderGroup and
> + BuilderCategory are already in use here for slightly different kinds of
> + grouping.
> + """
> + def __init__(self, builders):
> + self.virtualized = builders[0].virtualized
> + self.processors = builders[0].processors
> + self.builders = builders
> +
> +
> class BuilderGroup:
> """A group of builders for the processor.
>
>
> === modified file 'lib/lp/buildmaster/templates/builders-index.pt'
> --- lib/lp/buildmaster/templates/builders-index.pt 2013-12-03 04:51:48 +0000
> +++ lib/lp/buildmaster/templates/builders-index.pt 2015-03-23 12:07:22 +0000
> @@ -44,44 +44,42 @@
> <div class="first yui-u">
> <div id="builders-list" class="portlet">
> <h2>Builders</h2>
> - <table class="listing sortable" id="builders-table">
> + <table class="listing" id="builders-table">
> <thead>
> <tr>
> <th></th>
> <th>Name</th>
> - <th>Architectures</th>
> <th>Status</th>
> </tr>
> </thead>
> <tbody>
> - <tr class="builder-row" tal:repeat="builder view/builders">
> - <td class="icon left">
> - <span class="sortkey" tal:content="builder/virtualized" />
> - <img src="/@@/ppa-icon" alt="[ppa]" title="PPA builder"
> - tal:condition="builder/virtualized" />
> - <img src="/@@/distribution" alt="[official]"
> - title="Official builder"
> - tal:condition="not: builder/virtualized" />
> - </td>
> - <td style="width: 9em;">
> - <span class="sortkey" tal:content="builder/name" />
> - <a tal:attributes="href builder/fmt:url"
> - tal:content="builder/name">Bob</a>
> - </td>
> - <td style="width: 9em;">
> - <span class="sortkey">
> - <tal:processor repeat="processor builder/processors"
> - content="processor/name">386</tal:processor>
> - </span>
> - <tal:repeat repeat="processor builder/processors">
> - <tal:processor replace="processor/name">386</tal:processor>
> - </tal:repeat>
> - </td>
> - <td tal:define="job builder/currentjob">
> - <metal:status-summary
> - use-macro="builder/@@+index/status-summary" />
> - </td>
> - </tr>
> + <tal:clump tal:repeat="clump view/builder_clumps">
> + <tr class="category">
> + <td class="icon left">
> + <img src="/@@/ppa-icon" alt="[virtual]"
> + title="Virtual builder"
> + tal:condition="clump/virtualized" />
> + <img src="/@@/distribution" alt="[non-virtual]"
> + title="Non-virtual builder"
> + tal:condition="not: clump/virtualized" />
> + </td>
> + <td colspan="2">
> + <tal:processor repeat="processor clump/processors"
> + content="processor/name" />
> + </td>
> + </tr>
> + <tr class="builder-row" tal:repeat="builder clump/builders">
> + <td></td>
> + <td style="width: 9em;">
> + <a tal:attributes="href builder/fmt:url"
> + tal:content="builder/name">Bob</a>
> + </td>
> + <td tal:define="job builder/currentjob">
> + <metal:status-summary
> + use-macro="builder/@@+index/status-summary" />
> + </td>
> + </tr>
> + </tal:clump>
> </tbody>
> </table>
> <p tal:condition="view/user">
> @@ -92,7 +90,7 @@
> </div><!-- yui-u -->
>
> <div class="yui-u">
> - <div id="official-queue-status" class="portlet">
> + <div id="nonvirt-queue-status" class="portlet">
> <div tal:define="category view/nonvirt_builders">
> <div metal:use-macro="template/macros/builder-category" />
> </div>
> @@ -100,7 +98,7 @@
> </div><!-- yui-u -->
>
> <div class="yui-u">
> - <div id="ppa-queue-status" class="portlet">
> + <div id="virt-queue-status" class="portlet">
> <div tal:define="category view/virt_builders">
> <div metal:use-macro="template/macros/builder-category" />
> </div>
>
> === modified file 'lib/lp/soyuz/browser/tests/builder-views.txt'
> --- lib/lp/soyuz/browser/tests/builder-views.txt 2014-06-20 12:21:49 +0000
> +++ lib/lp/soyuz/browser/tests/builder-views.txt 2015-03-23 12:07:22 +0000
> @@ -234,11 +234,11 @@
> BuilderSetView offer a way to treat the currently registered builders
> in categories. They are:
>
> - * 'Official distribution build machines': a group of builders capable
> - of building 'trusted' sources, ubuntu official packages. The
> - 'non-vitualized' build-farm.
> + * 'Non-virtual build machines': a group of builders capable of building
> + 'trusted' sources, Ubuntu official packages. The 'non-virtualized'
> + build-farm.
>
> - * 'PPA build machines': a group of builders capable of building
> + * 'Virtual build machines': a group of builders capable of building
> 'untrusted' sources, PPA packages. The 'virtualized' build-farm.
>
> >>> from lp.buildmaster.interfaces.builder import IBuilderSet
> @@ -296,7 +296,7 @@
> <...BuilderCategory ...>
>
> >>> print builder_category.title
> - Official distributions build status
> + Non-virtual build status
>
> >>> print builder_category.virtualized
> False
> @@ -346,13 +346,13 @@
> >>> print i386_group.duration
> 0:00:30
>
> -The 'PPA' builder category is also available in BuilderSetView as a
> +The 'virtual' builder category is also available in BuilderSetView as a
> `BuilderCategory`.
>
> >>> builder_category = builderset_view.virt_builders
>
> >>> print builder_category.title
> - PPA build status
> + Virtual build status
>
> >>> print builder_category.virtualized
> True
>
> === modified file 'lib/lp/soyuz/stories/soyuz/xx-builder-page.txt'
> --- lib/lp/soyuz/stories/soyuz/xx-builder-page.txt 2014-06-20 07:59:21 +0000
> +++ lib/lp/soyuz/stories/soyuz/xx-builder-page.txt 2015-03-23 12:07:22 +0000
> @@ -261,13 +261,14 @@
> 1 available build machine, 1 disabled and 0 building of a total
> of 2 registered.
> Builders
> - Name Architectures Status
> - frog 386 Disabled
> - victim 386 Idle
> + Name Status
> + 386
> + frog Disabled
> + victim Idle
> Updated on ...
> - Official distributions build status
> + Non-virtual build status
> Architecture Builders Queue
> - PPA build status
> + Virtual build status
> Architecture Builders Queue
> 386 1 empty
>
>
> === modified file 'lib/lp/soyuz/stories/soyuz/xx-buildfarm-index.txt'
> --- lib/lp/soyuz/stories/soyuz/xx-buildfarm-index.txt 2014-06-20 08:32:31 +0000
> +++ lib/lp/soyuz/stories/soyuz/xx-buildfarm-index.txt 2015-03-23 12:07:22 +0000
> @@ -8,7 +8,7 @@
> >>> anon_browser.open('http://launchpad.dev/+builds')
>
> The BuildFarm contains a list of all builders registered in Launchpad
> -ordered by build domain ('official' then 'ppa'), then processor and
> +ordered by build domain ('nonvirt' then 'virt'), then processor and
> finally by name. A short textual description of their status is listed
> in the 'Status' column. There are also 2 portlets on the right-side
> containing the build queue status summary for each build domain.
> @@ -18,13 +18,14 @@
> 1 available build machine, 1 disabled and 1 building of a total of
> 2 registered.
> Builders
> - Name Architectures Status
> - bob 386 Building i386 build of mozilla-firefox 0.9 ...
> - frog 386 Disabled
> - Official distributions build status
> + Name Status
> + 386
> + bob Building i386 build of mozilla-firefox 0.9 ...
> + frog Disabled
> + Non-virtual build status
> Architecture Builders Queue
> 386 1 1 job (1 minute)
> - PPA build status
> + Virtual build status
> Architecture Builders Queue
> 386 0 empty
>
> @@ -37,26 +38,26 @@
>
> The build status portlets contain the number of builds waiting
> in queue and the sum of their 'estimated_duration' for each
> -supported processor on each separated build domain, 'official'
> -(PRIMARY and PARTNER) and 'ppa'.
> +supported processor on each separated build domain, 'nonvirt'
> +(PRIMARY and PARTNER) and 'virt'.
>
> >>> anon_browser.reload()
> >>> print extract_text(
> - ... find_tag_by_id(anon_browser.contents, 'official-queue-status'))
> - Official distributions build status
> + ... find_tag_by_id(anon_browser.contents, 'nonvirt-queue-status'))
> + Non-virtual build status
> Architecture Builders Queue
> 386 1 1 job (1 minute)
>
> >>> print extract_text(
> - ... find_tag_by_id(anon_browser.contents, 'ppa-queue-status',))
> - PPA build status
> + ... find_tag_by_id(anon_browser.contents, 'virt-queue-status',))
> + Virtual build status
> Architecture Builders Queue
> 386 0 empty
>
> -When there are pending builds in the PPA domain, they are listed in
> +When there are pending builds in the virt domain, they are listed in
> the corresponding portlet with their estimated duration as well.
>
> - # Retry a failed build in the PPA domain and set its
> + # Retry a failed build in the virt domain and set its
> # 'estimated_duration' to a known value.
> >>> import datetime
> >>> from zope.component import getUtility
> @@ -75,13 +76,13 @@
>
> >>> anon_browser.reload()
> >>> print extract_text(
> - ... find_tag_by_id(anon_browser.contents, 'ppa-queue-status'))
> - PPA build status
> + ... find_tag_by_id(anon_browser.contents, 'virt-queue-status'))
> + Virtual build status
> Architecture Builders Queue
> 386 0 1 job (1 minute)
>
> If the archive for the build does not require virtual builders, then
> -the pending job will appear in the 'official' queue. Since the build
> +the pending job will appear in the 'nonvirt' queue. Since the build
> record already exists, we must manually set it to non-virtualized too.
>
> >>> login('foo.bar@xxxxxxxxxxxxx')
> @@ -91,14 +92,14 @@
>
> >>> anon_browser.reload()
> >>> print extract_text(
> - ... find_tag_by_id(anon_browser.contents, 'ppa-queue-status'))
> - PPA build status
> + ... find_tag_by_id(anon_browser.contents, 'virt-queue-status'))
> + Virtual build status
> Architecture Builders Queue
> 386 0 empty
>
> >>> print extract_text(
> - ... find_tag_by_id(anon_browser.contents, 'official-queue-status'))
> - Official distributions build status
> + ... find_tag_by_id(anon_browser.contents, 'nonvirt-queue-status'))
> + Non-virtual build status
> Architecture Builders Queue
> 386 1 2 jobs (2 minutes)
>
>
--
https://code.launchpad.net/~cjwatson/launchpad/rearrange-builders/+merge/253818
Your team Launchpad code reviewers is subscribed to branch lp:launchpad.
References
