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
=== modified file 'lib/lp/soyuz/model/archive.py'
--- lib/lp/soyuz/model/archive.py 2010-08-03 14:59:22 +0000
+++ lib/lp/soyuz/model/archive.py 2010-08-04 22:27:45 +0000
@@ -78,7 +78,6 @@
78 ArchivePermissionType, IArchivePermissionSet)78 ArchivePermissionType, IArchivePermissionSet)
79from lp.soyuz.interfaces.archivesubscriber import (79from lp.soyuz.interfaces.archivesubscriber import (
80 ArchiveSubscriberStatus, IArchiveSubscriberSet, ArchiveSubscriptionError)80 ArchiveSubscriberStatus, IArchiveSubscriberSet, ArchiveSubscriptionError)
81from lp.soyuz.interfaces.binarypackagerelease import BinaryPackageFileType
82from lp.soyuz.interfaces.binarypackagebuild import IBinaryPackageBuildSet81from lp.soyuz.interfaces.binarypackagebuild import IBinaryPackageBuildSet
83from lp.soyuz.interfaces.buildrecords import (82from lp.soyuz.interfaces.buildrecords import (
84 IHasBuildRecords, IncompatibleArguments)83 IHasBuildRecords, IncompatibleArguments)
@@ -336,9 +335,9 @@
336 def archive_url(self):335 def archive_url(self):
337 """See `IArchive`."""336 """See `IArchive`."""
338 archive_postfixes = {337 archive_postfixes = {
339 ArchivePurpose.PRIMARY : '',338 ArchivePurpose.PRIMARY: '',
340 ArchivePurpose.PARTNER : '-partner',339 ArchivePurpose.PARTNER: '-partner',
341 ArchivePurpose.DEBUG : '-debug',340 ArchivePurpose.DEBUG: '-debug',
342 }341 }
343342
344 if self.is_ppa:343 if self.is_ppa:
@@ -431,7 +430,7 @@
431 try:430 try:
432 status = tuple(status)431 status = tuple(status)
433 except TypeError:432 except TypeError:
434 status = (status,)433 status = (status, )
435 clauses.append("""434 clauses.append("""
436 SourcePackagePublishingHistory.status IN %s435 SourcePackagePublishingHistory.status IN %s
437 """ % sqlvalues(status))436 """ % sqlvalues(status))
@@ -501,7 +500,7 @@
501 try:500 try:
502 status = tuple(status)501 status = tuple(status)
503 except TypeError:502 except TypeError:
504 status = (status,)503 status = (status, )
505 clauses.append("""504 clauses.append("""
506 SourcePackagePublishingHistory.status IN %s505 SourcePackagePublishingHistory.status IN %s
507 """ % sqlvalues(status))506 """ % sqlvalues(status))
@@ -543,7 +542,7 @@
543 result = store.find((542 result = store.find((
544 LibraryFileAlias.filename,543 LibraryFileAlias.filename,
545 LibraryFileContent.sha1,544 LibraryFileContent.sha1,
546 LibraryFileContent.filesize,),545 LibraryFileContent.filesize),
547 SourcePackagePublishingHistory.archive == self.id,546 SourcePackagePublishingHistory.archive == self.id,
548 SourcePackagePublishingHistory.dateremoved == None,547 SourcePackagePublishingHistory.dateremoved == None,
549 SourcePackagePublishingHistory.sourcepackagereleaseID ==548 SourcePackagePublishingHistory.sourcepackagereleaseID ==
@@ -568,7 +567,7 @@
568 # the result is empty, so instead:567 # the result is empty, so instead:
569 return sum(result.values(LibraryFileContent.filesize))568 return sum(result.values(LibraryFileContent.filesize))
570569
571 def _getBinaryPublishingBaseClauses (570 def _getBinaryPublishingBaseClauses(
572 self, name=None, version=None, status=None, distroarchseries=None,571 self, name=None, version=None, status=None, distroarchseries=None,
573 pocket=None, exact_match=False):572 pocket=None, exact_match=False):
574 """Base clauses and clauseTables for binary publishing queries.573 """Base clauses and clauseTables for binary publishing queries.
@@ -615,7 +614,7 @@
615 try:614 try:
616 status = tuple(status)615 status = tuple(status)
617 except TypeError:616 except TypeError:
618 status = (status,)617 status = (status, )
619 clauses.append("""618 clauses.append("""
620 BinaryPackagePublishingHistory.status IN %s619 BinaryPackagePublishingHistory.status IN %s
621 """ % sqlvalues(status))620 """ % sqlvalues(status))
@@ -624,7 +623,7 @@
624 try:623 try:
625 distroarchseries = tuple(distroarchseries)624 distroarchseries = tuple(distroarchseries)
626 except TypeError:625 except TypeError:
627 distroarchseries = (distroarchseries,)626 distroarchseries = (distroarchseries, )
628 # XXX cprov 20071016: there is no sqlrepr for DistroArchSeries627 # XXX cprov 20071016: there is no sqlrepr for DistroArchSeries
629 # uhmm, how so ?628 # uhmm, how so ?
630 das_ids = "(%s)" % ", ".join(str(d.id) for d in distroarchseries)629 das_ids = "(%s)" % ", ".join(str(d.id) for d in distroarchseries)
@@ -648,7 +647,7 @@
648 distroarchseries=distroarchseries, exact_match=exact_match)647 distroarchseries=distroarchseries, exact_match=exact_match)
649648
650 all_binaries = BinaryPackagePublishingHistory.select(649 all_binaries = BinaryPackagePublishingHistory.select(
651 ' AND '.join(clauses) , clauseTables=clauseTables,650 ' AND '.join(clauses), clauseTables=clauseTables,
652 orderBy=orderBy)651 orderBy=orderBy)
653652
654 return all_binaries653 return all_binaries
@@ -720,15 +719,8 @@
720 BinaryPackagePublishingHistory.binarypackagereleaseID ==719 BinaryPackagePublishingHistory.binarypackagereleaseID ==
721 BinaryPackageFile.binarypackagereleaseID,720 BinaryPackageFile.binarypackagereleaseID,
722 BinaryPackageFile.libraryfileID == LibraryFileAlias.id,721 BinaryPackageFile.libraryfileID == LibraryFileAlias.id,
723 LibraryFileAlias.contentID == LibraryFileContent.id722 LibraryFileAlias.contentID == LibraryFileContent.id,
724 ]723 ]
725
726 # Exclude DDEBs from the repository size, they are not published
727 # on disk for PPAs. See bug #399444 for more information.
728 if self.is_ppa:
729 clauses.append(
730 BinaryPackageFile.filetype != BinaryPackageFileType.DDEB)
731
732 result = store.find(LibraryFileContent, *clauses)724 result = store.find(LibraryFileContent, *clauses)
733725
734 # See `IArchive.sources_size`.726 # See `IArchive.sources_size`.
@@ -750,10 +742,10 @@
750 def allowUpdatesToReleasePocket(self):742 def allowUpdatesToReleasePocket(self):
751 """See `IArchive`."""743 """See `IArchive`."""
752 purposeToPermissionMap = {744 purposeToPermissionMap = {
753 ArchivePurpose.COPY : True,745 ArchivePurpose.COPY: True,
754 ArchivePurpose.PARTNER : True,746 ArchivePurpose.PARTNER: True,
755 ArchivePurpose.PPA : True,747 ArchivePurpose.PPA: True,
756 ArchivePurpose.PRIMARY : False,748 ArchivePurpose.PRIMARY: False,
757 }749 }
758750
759 try:751 try:
@@ -914,7 +906,7 @@
914906
915 find_spec = (907 find_spec = (
916 BuildFarmJob.status,908 BuildFarmJob.status,
917 Count(BinaryPackageBuild.id)909 Count(BinaryPackageBuild.id),
918 )910 )
919 result = store.using(911 result = store.using(
920 BinaryPackageBuild, PackageBuild, BuildFarmJob).find(912 BinaryPackageBuild, PackageBuild, BuildFarmJob).find(
@@ -955,7 +947,7 @@
955 BuildStatus.BUILDING,947 BuildStatus.BUILDING,
956 BuildStatus.FULLYBUILT,948 BuildStatus.FULLYBUILT,
957 BuildStatus.SUPERSEDED,949 BuildStatus.SUPERSEDED,
958 ]950 ],
959 }951 }
960952
961 # If we were asked to include builds with the state NEEDSBUILD,953 # If we were asked to include builds with the state NEEDSBUILD,
@@ -1668,7 +1660,6 @@
16681660
1669 return default_name_by_purpose[purpose]1661 return default_name_by_purpose[purpose]
16701662
1671
1672 def getByDistroPurpose(self, distribution, purpose, name=None):1663 def getByDistroPurpose(self, distribution, purpose, name=None):
1673 """See `IArchiveSet`."""1664 """See `IArchiveSet`."""
1674 if purpose == ArchivePurpose.PPA:1665 if purpose == ArchivePurpose.PPA:
@@ -1777,7 +1768,6 @@
17771768
1778 return new_archive1769 return new_archive
17791770
1780
1781 def __iter__(self):1771 def __iter__(self):
1782 """See `IArchiveSet`."""1772 """See `IArchiveSet`."""
1783 return iter(Archive.select())1773 return iter(Archive.select())
@@ -1852,7 +1842,8 @@
1852 origin = (1842 origin = (
1853 Archive,1843 Archive,
1854 Join(SourcePackagePublishingHistory,1844 Join(SourcePackagePublishingHistory,
1855 SourcePackagePublishingHistory.archive == Archive.id),)1845 SourcePackagePublishingHistory.archive == Archive.id),
1846 )
1856 results = store.using(*origin).find(1847 results = store.using(*origin).find(
1857 Archive,1848 Archive,
1858 Archive.signing_key == None,1849 Archive.signing_key == None,
@@ -1977,7 +1968,7 @@
1977 # If a single purpose is passed in, convert it into a tuple,1968 # If a single purpose is passed in, convert it into a tuple,
1978 # otherwise assume a list was passed in.1969 # otherwise assume a list was passed in.
1979 if purposes in ArchivePurpose:1970 if purposes in ArchivePurpose:
1980 purposes = (purposes,)1971 purposes = (purposes, )
19811972
1982 if purposes:1973 if purposes:
1983 extra_exprs.append(Archive.purpose.is_in(purposes))1974 extra_exprs.append(Archive.purpose.is_in(purposes))
19841975
=== modified file 'lib/lp/soyuz/tests/test_archive.py'
--- lib/lp/soyuz/tests/test_archive.py 2010-08-03 19:10:42 +0000
+++ lib/lp/soyuz/tests/test_archive.py 2010-08-04 22:27:45 +0000
@@ -27,8 +27,6 @@
27from lp.services.worlddata.interfaces.country import ICountrySet27from lp.services.worlddata.interfaces.country import ICountrySet
28from lp.soyuz.interfaces.archivearch import IArchiveArchSet28from lp.soyuz.interfaces.archivearch import IArchiveArchSet
29from lp.soyuz.interfaces.binarypackagename import IBinaryPackageNameSet29from lp.soyuz.interfaces.binarypackagename import IBinaryPackageNameSet
30from lp.soyuz.interfaces.binarypackagerelease import (
31 BinaryPackageFileType, BinaryPackageFormat)
32from lp.soyuz.interfaces.component import IComponentSet30from lp.soyuz.interfaces.component import IComponentSet
33from lp.soyuz.interfaces.processor import IProcessorFamilySet31from lp.soyuz.interfaces.processor import IProcessorFamilySet
34from lp.soyuz.interfaces.publishing import PackagePublishingStatus32from lp.soyuz.interfaces.publishing import PackagePublishingStatus
@@ -131,45 +129,11 @@
131129
132 layer = LaunchpadZopelessLayer130 layer = LaunchpadZopelessLayer
133131
134 def publishDDEBInArchive(self, archive):
135 """Publish an arbitrary DDEB with content in to the archive.
136
137 Publishes a DDEB that will take up some space in to `archive`.
138
139 :param archive: the IArchive to publish to.
140 """
141 binarypackagerelease = self.factory.makeBinaryPackageRelease(
142 binpackageformat=BinaryPackageFormat.DDEB)
143 self.factory.makeBinaryPackagePublishingHistory(
144 archive=archive, binarypackagerelease=binarypackagerelease,
145 status=PackagePublishingStatus.PUBLISHED)
146 self.factory.makeBinaryPackageFile(
147 binarypackagerelease=binarypackagerelease,
148 filetype=BinaryPackageFileType.DDEB)
149
150 def test_empty_ppa_has_zero_binaries_size(self):132 def test_empty_ppa_has_zero_binaries_size(self):
151 # An empty PPA has no binaries so has zero binaries_size.133 # An empty PPA has no binaries so has zero binaries_size.
152 ppa = self.factory.makeArchive(purpose=ArchivePurpose.PPA)134 ppa = self.factory.makeArchive(purpose=ArchivePurpose.PPA)
153 self.assertEquals(0, ppa.binaries_size)135 self.assertEquals(0, ppa.binaries_size)
154136
155 def test_binaries_size_does_not_include_ddebs_for_ppas(self):
156 # DDEBs are not computed in the PPA binaries size because
157 # they are not being published. See bug #399444.
158 ppa = self.factory.makeArchive(purpose=ArchivePurpose.PPA)
159 self.publishDDEBInArchive(ppa)
160 self.assertEquals(0, ppa.binaries_size)
161
162 def test_empty_primary_archive_has_zero_binaries_size(self):
163 # PRIMARY archives have zero binaries_size when created.
164 archive = self.factory.makeArchive(purpose=ArchivePurpose.PRIMARY)
165 self.assertEquals(0, archive.binaries_size)
166
167 def test_binaries_size_includes_ddebs_for_other_archives(self):
168 # DDEBs size are computed for all archive purposes, except PPAs.
169 archive = self.factory.makeArchive(purpose=ArchivePurpose.PRIMARY)
170 self.publishDDEBInArchive(archive)
171 self.assertNotEquals(0, archive.binaries_size)
172
173 def test_sources_size_on_empty_archive(self):137 def test_sources_size_on_empty_archive(self):
174 # Zero is returned for an archive without sources.138 # Zero is returned for an archive without sources.
175 archive = self.factory.makeArchive()139 archive = self.factory.makeArchive()