Launchpad itself

Merge lp:~wgrant/launchpad/bug-612157-ppa-quota-ddebs into lp:launchpad

  1. bug-612157-ppa-quota-ddebs
  2. Merge into devel
Proposed by William Grant
Status: Merged
Approved by: Māris Fogels
Approved revision: no longer in the source branch.
Merged at revision: 11297
Proposed branch: lp:~wgrant/launchpad/bug-612157-ppa-quota-ddebs
Merge into: lp:launchpad
Diff against target: 231 lines (+20/-65)
2 files modified
lib/lp/soyuz/model/archive.py (+20/-29)
lib/lp/soyuz/tests/test_archive.py (+0/-36)
To merge this branch: bzr merge lp:~wgrant/launchpad/bug-612157-ppa-quota-ddebs
Reviewer Review Type Date Requested Status
Māris Fogels (community) code Approve
Review via email: mp+31471@code.launchpad.net

Commit message

Don't exclude ddebs from the PPA size calculation.

Description of the change

When reactivating PPA ddeb publication (fixing bug #604433), I failed to notice that the PPA size calculation excluded ddebs (bug #612157). This branch removes the special case and its tests, and fixes lint.

To post a comment you must log in.
Revision history for this message
Māris Fogels (mars) wrote :

Hi William,

This is a nice, simple change. One potential issue: you removed the requirement that DDEBS not be counted, and you removed the associated test - good. This means that DDEBS should now be included in the archive size, but I see no test asserting that this is true. A previously excluded number is now included in the sum total archive size - some test somewhere should have changed to reflect this.

But maybe not - if you feel that my worry is unfounded, then feel free to land this. The code looks good. r=mars

Maris

review: Approve (code)
Revision history for this message
William Grant (wgrant) wrote :

I considered that, but it was just removing a hackish special case. Should we really be testing the absence of every special case?

Revision history for this message
Robert Collins (lifeless) wrote :

No

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-08-03 14:59:22 +0000
3+++ lib/lp/soyuz/model/archive.py 2010-08-04 22:27:45 +0000
4@@ -78,7 +78,6 @@
5 ArchivePermissionType, IArchivePermissionSet)
6 from lp.soyuz.interfaces.archivesubscriber import (
7 ArchiveSubscriberStatus, IArchiveSubscriberSet, ArchiveSubscriptionError)
8-from lp.soyuz.interfaces.binarypackagerelease import BinaryPackageFileType
9 from lp.soyuz.interfaces.binarypackagebuild import IBinaryPackageBuildSet
10 from lp.soyuz.interfaces.buildrecords import (
11 IHasBuildRecords, IncompatibleArguments)
12@@ -336,9 +335,9 @@
13 def archive_url(self):
14 """See `IArchive`."""
15 archive_postfixes = {
16- ArchivePurpose.PRIMARY : '',
17- ArchivePurpose.PARTNER : '-partner',
18- ArchivePurpose.DEBUG : '-debug',
19+ ArchivePurpose.PRIMARY: '',
20+ ArchivePurpose.PARTNER: '-partner',
21+ ArchivePurpose.DEBUG: '-debug',
22 }
23
24 if self.is_ppa:
25@@ -431,7 +430,7 @@
26 try:
27 status = tuple(status)
28 except TypeError:
29- status = (status,)
30+ status = (status, )
31 clauses.append("""
32 SourcePackagePublishingHistory.status IN %s
33 """ % sqlvalues(status))
34@@ -501,7 +500,7 @@
35 try:
36 status = tuple(status)
37 except TypeError:
38- status = (status,)
39+ status = (status, )
40 clauses.append("""
41 SourcePackagePublishingHistory.status IN %s
42 """ % sqlvalues(status))
43@@ -543,7 +542,7 @@
44 result = store.find((
45 LibraryFileAlias.filename,
46 LibraryFileContent.sha1,
47- LibraryFileContent.filesize,),
48+ LibraryFileContent.filesize),
49 SourcePackagePublishingHistory.archive == self.id,
50 SourcePackagePublishingHistory.dateremoved == None,
51 SourcePackagePublishingHistory.sourcepackagereleaseID ==
52@@ -568,7 +567,7 @@
53 # the result is empty, so instead:
54 return sum(result.values(LibraryFileContent.filesize))
55
56- def _getBinaryPublishingBaseClauses (
57+ def _getBinaryPublishingBaseClauses(
58 self, name=None, version=None, status=None, distroarchseries=None,
59 pocket=None, exact_match=False):
60 """Base clauses and clauseTables for binary publishing queries.
61@@ -615,7 +614,7 @@
62 try:
63 status = tuple(status)
64 except TypeError:
65- status = (status,)
66+ status = (status, )
67 clauses.append("""
68 BinaryPackagePublishingHistory.status IN %s
69 """ % sqlvalues(status))
70@@ -624,7 +623,7 @@
71 try:
72 distroarchseries = tuple(distroarchseries)
73 except TypeError:
74- distroarchseries = (distroarchseries,)
75+ distroarchseries = (distroarchseries, )
76 # XXX cprov 20071016: there is no sqlrepr for DistroArchSeries
77 # uhmm, how so ?
78 das_ids = "(%s)" % ", ".join(str(d.id) for d in distroarchseries)
79@@ -648,7 +647,7 @@
80 distroarchseries=distroarchseries, exact_match=exact_match)
81
82 all_binaries = BinaryPackagePublishingHistory.select(
83- ' AND '.join(clauses) , clauseTables=clauseTables,
84+ ' AND '.join(clauses), clauseTables=clauseTables,
85 orderBy=orderBy)
86
87 return all_binaries
88@@ -720,15 +719,8 @@
89 BinaryPackagePublishingHistory.binarypackagereleaseID ==
90 BinaryPackageFile.binarypackagereleaseID,
91 BinaryPackageFile.libraryfileID == LibraryFileAlias.id,
92- LibraryFileAlias.contentID == LibraryFileContent.id
93+ LibraryFileAlias.contentID == LibraryFileContent.id,
94 ]
95-
96- # Exclude DDEBs from the repository size, they are not published
97- # on disk for PPAs. See bug #399444 for more information.
98- if self.is_ppa:
99- clauses.append(
100- BinaryPackageFile.filetype != BinaryPackageFileType.DDEB)
101-
102 result = store.find(LibraryFileContent, *clauses)
103
104 # See `IArchive.sources_size`.
105@@ -750,10 +742,10 @@
106 def allowUpdatesToReleasePocket(self):
107 """See `IArchive`."""
108 purposeToPermissionMap = {
109- ArchivePurpose.COPY : True,
110- ArchivePurpose.PARTNER : True,
111- ArchivePurpose.PPA : True,
112- ArchivePurpose.PRIMARY : False,
113+ ArchivePurpose.COPY: True,
114+ ArchivePurpose.PARTNER: True,
115+ ArchivePurpose.PPA: True,
116+ ArchivePurpose.PRIMARY: False,
117 }
118
119 try:
120@@ -914,7 +906,7 @@
121
122 find_spec = (
123 BuildFarmJob.status,
124- Count(BinaryPackageBuild.id)
125+ Count(BinaryPackageBuild.id),
126 )
127 result = store.using(
128 BinaryPackageBuild, PackageBuild, BuildFarmJob).find(
129@@ -955,7 +947,7 @@
130 BuildStatus.BUILDING,
131 BuildStatus.FULLYBUILT,
132 BuildStatus.SUPERSEDED,
133- ]
134+ ],
135 }
136
137 # If we were asked to include builds with the state NEEDSBUILD,
138@@ -1668,7 +1660,6 @@
139
140 return default_name_by_purpose[purpose]
141
142-
143 def getByDistroPurpose(self, distribution, purpose, name=None):
144 """See `IArchiveSet`."""
145 if purpose == ArchivePurpose.PPA:
146@@ -1777,7 +1768,6 @@
147
148 return new_archive
149
150-
151 def __iter__(self):
152 """See `IArchiveSet`."""
153 return iter(Archive.select())
154@@ -1852,7 +1842,8 @@
155 origin = (
156 Archive,
157 Join(SourcePackagePublishingHistory,
158- SourcePackagePublishingHistory.archive == Archive.id),)
159+ SourcePackagePublishingHistory.archive == Archive.id),
160+ )
161 results = store.using(*origin).find(
162 Archive,
163 Archive.signing_key == None,
164@@ -1977,7 +1968,7 @@
165 # If a single purpose is passed in, convert it into a tuple,
166 # otherwise assume a list was passed in.
167 if purposes in ArchivePurpose:
168- purposes = (purposes,)
169+ purposes = (purposes, )
170
171 if purposes:
172 extra_exprs.append(Archive.purpose.is_in(purposes))
173
174=== modified file 'lib/lp/soyuz/tests/test_archive.py'
175--- lib/lp/soyuz/tests/test_archive.py 2010-08-03 19:10:42 +0000
176+++ lib/lp/soyuz/tests/test_archive.py 2010-08-04 22:27:45 +0000
177@@ -27,8 +27,6 @@
178 from lp.services.worlddata.interfaces.country import ICountrySet
179 from lp.soyuz.interfaces.archivearch import IArchiveArchSet
180 from lp.soyuz.interfaces.binarypackagename import IBinaryPackageNameSet
181-from lp.soyuz.interfaces.binarypackagerelease import (
182- BinaryPackageFileType, BinaryPackageFormat)
183 from lp.soyuz.interfaces.component import IComponentSet
184 from lp.soyuz.interfaces.processor import IProcessorFamilySet
185 from lp.soyuz.interfaces.publishing import PackagePublishingStatus
186@@ -131,45 +129,11 @@
187
188 layer = LaunchpadZopelessLayer
189
190- def publishDDEBInArchive(self, archive):
191- """Publish an arbitrary DDEB with content in to the archive.
192-
193- Publishes a DDEB that will take up some space in to `archive`.
194-
195- :param archive: the IArchive to publish to.
196- """
197- binarypackagerelease = self.factory.makeBinaryPackageRelease(
198- binpackageformat=BinaryPackageFormat.DDEB)
199- self.factory.makeBinaryPackagePublishingHistory(
200- archive=archive, binarypackagerelease=binarypackagerelease,
201- status=PackagePublishingStatus.PUBLISHED)
202- self.factory.makeBinaryPackageFile(
203- binarypackagerelease=binarypackagerelease,
204- filetype=BinaryPackageFileType.DDEB)
205-
206 def test_empty_ppa_has_zero_binaries_size(self):
207 # An empty PPA has no binaries so has zero binaries_size.
208 ppa = self.factory.makeArchive(purpose=ArchivePurpose.PPA)
209 self.assertEquals(0, ppa.binaries_size)
210
211- def test_binaries_size_does_not_include_ddebs_for_ppas(self):
212- # DDEBs are not computed in the PPA binaries size because
213- # they are not being published. See bug #399444.
214- ppa = self.factory.makeArchive(purpose=ArchivePurpose.PPA)
215- self.publishDDEBInArchive(ppa)
216- self.assertEquals(0, ppa.binaries_size)
217-
218- def test_empty_primary_archive_has_zero_binaries_size(self):
219- # PRIMARY archives have zero binaries_size when created.
220- archive = self.factory.makeArchive(purpose=ArchivePurpose.PRIMARY)
221- self.assertEquals(0, archive.binaries_size)
222-
223- def test_binaries_size_includes_ddebs_for_other_archives(self):
224- # DDEBs size are computed for all archive purposes, except PPAs.
225- archive = self.factory.makeArchive(purpose=ArchivePurpose.PRIMARY)
226- self.publishDDEBInArchive(archive)
227- self.assertNotEquals(0, archive.binaries_size)
228-
229 def test_sources_size_on_empty_archive(self):
230 # Zero is returned for an archive without sources.
231 archive = self.factory.makeArchive()