Merge lp:~sinzui/launchpad/publishbinary-without-publications into lp:launchpad

Proposed by Curtis Hovey on 2012-11-15
Status: Merged
Approved by: j.c.sackett on 2012-11-15
Approved revision: no longer in the source branch.
Merged at revision: 16277
Proposed branch: lp:~sinzui/launchpad/publishbinary-without-publications
Merge into: lp:launchpad
Diff against target: 50 lines (+21/-1)
2 files modified
lib/lp/soyuz/model/publishing.py (+6/-1)
lib/lp/soyuz/tests/test_publishing.py (+15/-0)
To merge this branch: bzr merge lp:~sinzui/launchpad/publishbinary-without-publications
Reviewer Review Type Date Requested Status
j.c.sackett (community) 2012-11-15 Approve on 2012-11-15
Review via email: mp+134527@code.launchpad.net

Commit Message

Do not attempt to publish binaries for disabled DistroArchSeries.

Description of the Change

As seen in OOPS-0255df70569a8fcc9fe45c645713e04c,
PublishingSet.publishBinaries OOPSes due to bad SQL if there are no
publications to create. This mostly happens when an architecture is
newly disabled.

--------------------------------------------------------------------

RULES

    Pre-implementation: wgrant
    * PublishingSet.publishBinaries must return a empty list of the
      binaries are for a disabled DistroArchSeries.
    * The publishBinaries method is the only callsite for
      expand_binary_requests, which promises to exclude unused
      architectures. The helper function failed to consider that
      an architecture was used, but was later disabled.

QA

    * Ask a webops to
      * Visit https://qastaging.launchpad.net/ubuntu/precise/armel/+admin
      * Disabled armel
      * For qastaging,
        ./cronscripts/publish-ftpmaster.py -v -d ubuntu
      * Verify the log does not container oopses
      * Visit https://qastaging.launchpad.net/ubuntu/precise/armel/+admin
      * Enabled armel again.

LINT

    lib/lp/soyuz/model/publishing.py
    lib/lp/soyuz/tests/test_publishing.py

LoC

    I have a 3,000 line credit this week.

TEST

    ./bin/test -vv -t TestPublishBinaries lp.soyuz.tests.test_publishing

IMPLEMENTATION

I updated expand_binary_requests() to ignore disabled DistroArchSeries.
I then updated publishBinaries() to return early if the expanded
binaries is empty.
    lib/lp/soyuz/model/publishing.py
    lib/lp/soyuz/tests/test_publishing.py

To post a comment you must log in.
j.c.sackett (jcsackett) wrote :

Thanks, Curtis.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'lib/lp/soyuz/model/publishing.py'
2--- lib/lp/soyuz/model/publishing.py 2012-10-30 01:46:10 +0000
3+++ lib/lp/soyuz/model/publishing.py 2012-11-15 17:26:27 +0000
4@@ -1396,7 +1396,8 @@
5 else:
6 target_archs = archs
7 for target_arch in target_archs:
8- expanded.append((target_arch, bpr, overrides))
9+ if target_arch.enabled:
10+ expanded.append((target_arch, bpr, overrides))
11 return expanded
12
13
14@@ -1456,6 +1457,10 @@
15 # Expand the dict of binaries into a list of tuples including the
16 # architecture.
17 expanded = expand_binary_requests(distroseries, binaries)
18+ if len(expanded) == 0:
19+ # The binaries are for a disabled DistroArchSeries or for
20+ # an unsupported architecture.
21+ return []
22
23 # Find existing publications.
24 # We should really be able to just compare BPR.id, but
25
26=== modified file 'lib/lp/soyuz/tests/test_publishing.py'
27--- lib/lp/soyuz/tests/test_publishing.py 2012-06-19 17:09:47 +0000
28+++ lib/lp/soyuz/tests/test_publishing.py 2012-11-15 17:26:27 +0000
29@@ -1599,6 +1599,21 @@
30 set((target_das_a, target_das_b)),
31 set(bpph.distroarchseries for bpph in bpphs))
32
33+ def test_architecture_disabled(self):
34+ # An empty list is return if the DistroArchSeries was disabled.
35+ arch_tag = self.factory.getUniqueString('arch-')
36+ orig_das = self.factory.makeDistroArchSeries(
37+ architecturetag=arch_tag)
38+ target_das = self.factory.makeDistroArchSeries(
39+ architecturetag=arch_tag)
40+ build = self.factory.makeBinaryPackageBuild(distroarchseries=orig_das)
41+ bpr = self.factory.makeBinaryPackageRelease(
42+ build=build, architecturespecific=True)
43+ target_das.enabled = False
44+ args = self.makeArgs([bpr], target_das.distroseries)
45+ results = getUtility(IPublishingSet).publishBinaries(**args)
46+ self.assertEqual([], results)
47+
48 def test_does_not_duplicate(self):
49 # An attempt to copy something for a second time is ignored.
50 bpr = self.factory.makeBinaryPackageRelease()