Launchpad itself

Merge lp:~jelmer/launchpad/600153-qafix into lp:launchpad

  1. 600153-qafix
  2. Merge into devel
Proposed by Jelmer Vernooij
Status: Merged
Approved by: Brad Crittenden
Approved revision: no longer in the source branch.
Merged at revision: 11239
Proposed branch: lp:~jelmer/launchpad/600153-qafix
Merge into: lp:launchpad
Diff against target: 54 lines (+5/-8)
2 files modified
lib/lp/soyuz/model/archive.py (+3/-4)
lib/lp/soyuz/model/publishing.py (+2/-4)
To merge this branch: bzr merge lp:~jelmer/launchpad/600153-qafix
Reviewer Review Type Date Requested Status
Brad Crittenden (community) code Approve
Launchpad code reviewers from Canonical code Pending
Review via email: mp+30790@code.launchpad.net

Description of the change

This fixes the qa-badness of bug 600153. process-upload was manually looking at ArchiveArch to determine what restricted architectures the archive could build on rather than checking Archive.enabled_restricted_families which looks at ArchiveArch *and* does other magic (such as whitelisting main archives).

To post a comment you must log in.
Revision history for this message
Brad Crittenden (bac) wrote :

Hi Jelmer,

Please sort your import items alphabetically.

Based on the QA report this change looks good (though I can't claim deep enough knowledge to know for sure).

You didn't add or modify any tests for this change. Is this fix not testable? Would an appropriate test on the original branch have avoided the second round?

review: Needs Information (code)
Revision history for this message
Jelmer Vernooij (jelmer) wrote :

Hi Brad,

Thanks for the review.

Since this really is a layering issue (the original code was skipping a layer and looking at the raw data directly, but doing a bad job interpreting that data), I'm not sure if it would make sense to add a test. I don't want to put too much knowledge about how we determine what restricted architecture families are allowed into the publisher tests - that logic is already covered in the archive tests.

What do you think?

Revision history for this message
Brad Crittenden (bac) :
review: Approve (code)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'lib/lp/soyuz/model/archive.py'
2--- lib/lp/soyuz/model/archive.py 2010-07-18 00:56:16 +0000
3+++ lib/lp/soyuz/model/archive.py 2010-07-27 12:56:05 +0000
4@@ -15,10 +15,9 @@
5 from sqlobject import (
6 BoolCol, ForeignKey, IntCol, StringCol)
7 from sqlobject.sqlbuilder import SQLConstant
8-from storm.expr import Or, And, Select, Sum, In, Desc
9+from storm.expr import And, Desc, Or, Select, Sum
10 from storm.locals import Count, Join
11 from storm.store import Store
12-from storm.zope.interfaces import IResultSet
13 from zope.component import getUtility
14 from zope.event import notify
15 from zope.interface import alsoProvides, implements
16@@ -1594,7 +1593,7 @@
17 def _getEnabledRestrictedFamilies(self):
18 """Retrieve the restricted architecture families this archive can
19 build on."""
20- # Main archives are always allowed to build on restricted
21+ # Main archives are always allowed to build on restricted
22 # architectures.
23 if self.is_main:
24 return getUtility(IProcessorFamilySet).getRestricted()
25@@ -1606,7 +1605,7 @@
26 def _setEnabledRestrictedFamilies(self, value):
27 """Set the restricted architecture families this archive can
28 build on."""
29- # Main archives are always allowed to build on restricted
30+ # Main archives are always allowed to build on restricted
31 # architectures.
32 if self.is_main:
33 proc_family_set = getUtility(IProcessorFamilySet)
34
35=== modified file 'lib/lp/soyuz/model/publishing.py'
36--- lib/lp/soyuz/model/publishing.py 2010-07-21 14:14:54 +0000
37+++ lib/lp/soyuz/model/publishing.py 2010-07-27 12:56:05 +0000
38@@ -517,14 +517,12 @@
39 :param available_archs: Architectures to consider
40 :return: Sequence of `IDistroArch` instances.
41 """
42- associated_proc_families = [
43- archivearch.processorfamily for archivearch
44- in getUtility(IArchiveArchSet).getByArchive(self.archive)]
45 # Return all distroarches with unrestricted processor families or with
46 # processor families the archive is explicitly associated with.
47 return [distroarch for distroarch in available_archs
48 if not distroarch.processorfamily.restricted or
49- distroarch.processorfamily in associated_proc_families]
50+ distroarch.processorfamily in
51+ self.archive.enabled_restricted_families]
52
53 def createMissingBuilds(self, architectures_available=None,
54 pas_verify=None, logger=None):