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
=== modified file 'lib/lp/soyuz/model/archive.py'
--- lib/lp/soyuz/model/archive.py 2010-07-18 00:56:16 +0000
+++ lib/lp/soyuz/model/archive.py 2010-07-27 12:56:05 +0000
@@ -15,10 +15,9 @@
15from sqlobject import (15from sqlobject import (
16 BoolCol, ForeignKey, IntCol, StringCol)16 BoolCol, ForeignKey, IntCol, StringCol)
17from sqlobject.sqlbuilder import SQLConstant17from sqlobject.sqlbuilder import SQLConstant
18from storm.expr import Or, And, Select, Sum, In, Desc18from storm.expr import And, Desc, Or, Select, Sum
19from storm.locals import Count, Join19from storm.locals import Count, Join
20from storm.store import Store20from storm.store import Store
21from storm.zope.interfaces import IResultSet
22from zope.component import getUtility21from zope.component import getUtility
23from zope.event import notify22from zope.event import notify
24from zope.interface import alsoProvides, implements23from zope.interface import alsoProvides, implements
@@ -1594,7 +1593,7 @@
1594 def _getEnabledRestrictedFamilies(self):1593 def _getEnabledRestrictedFamilies(self):
1595 """Retrieve the restricted architecture families this archive can1594 """Retrieve the restricted architecture families this archive can
1596 build on."""1595 build on."""
1597 # Main archives are always allowed to build on restricted 1596 # Main archives are always allowed to build on restricted
1598 # architectures.1597 # architectures.
1599 if self.is_main:1598 if self.is_main:
1600 return getUtility(IProcessorFamilySet).getRestricted()1599 return getUtility(IProcessorFamilySet).getRestricted()
@@ -1606,7 +1605,7 @@
1606 def _setEnabledRestrictedFamilies(self, value):1605 def _setEnabledRestrictedFamilies(self, value):
1607 """Set the restricted architecture families this archive can1606 """Set the restricted architecture families this archive can
1608 build on."""1607 build on."""
1609 # Main archives are always allowed to build on restricted 1608 # Main archives are always allowed to build on restricted
1610 # architectures.1609 # architectures.
1611 if self.is_main:1610 if self.is_main:
1612 proc_family_set = getUtility(IProcessorFamilySet)1611 proc_family_set = getUtility(IProcessorFamilySet)
16131612
=== modified file 'lib/lp/soyuz/model/publishing.py'
--- lib/lp/soyuz/model/publishing.py 2010-07-21 14:14:54 +0000
+++ lib/lp/soyuz/model/publishing.py 2010-07-27 12:56:05 +0000
@@ -517,14 +517,12 @@
517 :param available_archs: Architectures to consider517 :param available_archs: Architectures to consider
518 :return: Sequence of `IDistroArch` instances.518 :return: Sequence of `IDistroArch` instances.
519 """519 """
520 associated_proc_families = [
521 archivearch.processorfamily for archivearch
522 in getUtility(IArchiveArchSet).getByArchive(self.archive)]
523 # Return all distroarches with unrestricted processor families or with520 # Return all distroarches with unrestricted processor families or with
524 # processor families the archive is explicitly associated with.521 # processor families the archive is explicitly associated with.
525 return [distroarch for distroarch in available_archs522 return [distroarch for distroarch in available_archs
526 if not distroarch.processorfamily.restricted or523 if not distroarch.processorfamily.restricted or
527 distroarch.processorfamily in associated_proc_families]524 distroarch.processorfamily in
525 self.archive.enabled_restricted_families]
528526
529 def createMissingBuilds(self, architectures_available=None,527 def createMissingBuilds(self, architectures_available=None,
530 pas_verify=None, logger=None):528 pas_verify=None, logger=None):