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

[William Grant]

Don't exclude ddebs from the PPA size calculation.

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.

[Māris Fogels (mars)]

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)

[William Grant (wgrant)]

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

[Robert Collins (lifeless)]

No

=== 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 @@
     ArchivePermissionType, IArchivePermissionSet)
 from lp.soyuz.interfaces.archivesubscriber import (
     ArchiveSubscriberStatus, IArchiveSubscriberSet, ArchiveSubscriptionError)
-from lp.soyuz.interfaces.binarypackagerelease import BinaryPackageFileType
 from lp.soyuz.interfaces.binarypackagebuild import IBinaryPackageBuildSet
 from lp.soyuz.interfaces.buildrecords import (
     IHasBuildRecords, IncompatibleArguments)
@@ -336,9 +335,9 @@
     def archive_url(self):
         """See `IArchive`."""
         archive_postfixes = {
-            ArchivePurpose.PRIMARY : '',
-            ArchivePurpose.PARTNER : '-partner',
-            ArchivePurpose.DEBUG : '-debug',
+            ArchivePurpose.PRIMARY: '',
+            ArchivePurpose.PARTNER: '-partner',
+            ArchivePurpose.DEBUG: '-debug',
         }
 
         if self.is_ppa:
@@ -431,7 +430,7 @@
             try:
                 status = tuple(status)
             except TypeError:
-                status = (status,)
+                status = (status, )
             clauses.append("""
                 SourcePackagePublishingHistory.status IN %s
             """ % sqlvalues(status))
@@ -501,7 +500,7 @@
             try:
                 status = tuple(status)
             except TypeError:
-                status = (status,)
+                status = (status, )
             clauses.append("""
                 SourcePackagePublishingHistory.status IN %s
             """ % sqlvalues(status))
@@ -543,7 +542,7 @@
         result = store.find((
             LibraryFileAlias.filename,
             LibraryFileContent.sha1,
-            LibraryFileContent.filesize,),
+            LibraryFileContent.filesize),
             SourcePackagePublishingHistory.archive == self.id,
             SourcePackagePublishingHistory.dateremoved == None,
             SourcePackagePublishingHistory.sourcepackagereleaseID ==
@@ -568,7 +567,7 @@
         # the result is empty, so instead:
         return sum(result.values(LibraryFileContent.filesize))
 
-    def _getBinaryPublishingBaseClauses (
+    def _getBinaryPublishingBaseClauses(
         self, name=None, version=None, status=None, distroarchseries=None,
         pocket=None, exact_match=False):
         """Base clauses and clauseTables for binary publishing queries.
@@ -615,7 +614,7 @@
             try:
                 status = tuple(status)
             except TypeError:
-                status = (status,)
+                status = (status, )
             clauses.append("""
                 BinaryPackagePublishingHistory.status IN %s
             """ % sqlvalues(status))
@@ -624,7 +623,7 @@
             try:
                 distroarchseries = tuple(distroarchseries)
             except TypeError:
-                distroarchseries = (distroarchseries,)
+                distroarchseries = (distroarchseries, )
             # XXX cprov 20071016: there is no sqlrepr for DistroArchSeries
             # uhmm, how so ?
             das_ids = "(%s)" % ", ".join(str(d.id) for d in distroarchseries)
@@ -648,7 +647,7 @@
             distroarchseries=distroarchseries, exact_match=exact_match)
 
         all_binaries = BinaryPackagePublishingHistory.select(
-            ' AND '.join(clauses) , clauseTables=clauseTables,
+            ' AND '.join(clauses), clauseTables=clauseTables,
             orderBy=orderBy)
 
         return all_binaries
@@ -720,15 +719,8 @@
             BinaryPackagePublishingHistory.binarypackagereleaseID ==
                 BinaryPackageFile.binarypackagereleaseID,
             BinaryPackageFile.libraryfileID == LibraryFileAlias.id,
-            LibraryFileAlias.contentID == LibraryFileContent.id
+            LibraryFileAlias.contentID == LibraryFileContent.id,
             ]
-
-        # Exclude DDEBs from the repository size, they are not published
-        # on disk for PPAs. See bug #399444 for more information.
-        if self.is_ppa:
-            clauses.append(
-                BinaryPackageFile.filetype != BinaryPackageFileType.DDEB)
-
         result = store.find(LibraryFileContent, *clauses)
 
         # See `IArchive.sources_size`.
@@ -750,10 +742,10 @@
     def allowUpdatesToReleasePocket(self):
         """See `IArchive`."""
         purposeToPermissionMap = {
-            ArchivePurpose.COPY : True,
-            ArchivePurpose.PARTNER : True,
-            ArchivePurpose.PPA : True,
-            ArchivePurpose.PRIMARY : False,
+            ArchivePurpose.COPY: True,
+            ArchivePurpose.PARTNER: True,
+            ArchivePurpose.PPA: True,
+            ArchivePurpose.PRIMARY: False,
         }
 
         try:
@@ -914,7 +906,7 @@
 
         find_spec = (
             BuildFarmJob.status,
-            Count(BinaryPackageBuild.id)
+            Count(BinaryPackageBuild.id),
             )
         result = store.using(
             BinaryPackageBuild, PackageBuild, BuildFarmJob).find(
@@ -955,7 +947,7 @@
                 BuildStatus.BUILDING,
                 BuildStatus.FULLYBUILT,
                 BuildStatus.SUPERSEDED,
-                ]
+                ],
             }
 
         # If we were asked to include builds with the state NEEDSBUILD,
@@ -1668,7 +1660,6 @@
 
         return default_name_by_purpose[purpose]
 
-
     def getByDistroPurpose(self, distribution, purpose, name=None):
         """See `IArchiveSet`."""
         if purpose == ArchivePurpose.PPA:
@@ -1777,7 +1768,6 @@
 
         return new_archive
 
-
     def __iter__(self):
         """See `IArchiveSet`."""
         return iter(Archive.select())
@@ -1852,7 +1842,8 @@
         origin = (
             Archive,
             Join(SourcePackagePublishingHistory,
-                 SourcePackagePublishingHistory.archive == Archive.id),)
+                 SourcePackagePublishingHistory.archive == Archive.id),
+            )
         results = store.using(*origin).find(
             Archive,
             Archive.signing_key == None,
@@ -1977,7 +1968,7 @@
         # If a single purpose is passed in, convert it into a tuple,
         # otherwise assume a list was passed in.
         if purposes in ArchivePurpose:
-            purposes = (purposes,)
+            purposes = (purposes, )
 
         if purposes:
             extra_exprs.append(Archive.purpose.is_in(purposes))

=== 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 @@
 from lp.services.worlddata.interfaces.country import ICountrySet
 from lp.soyuz.interfaces.archivearch import IArchiveArchSet
 from lp.soyuz.interfaces.binarypackagename import IBinaryPackageNameSet
-from lp.soyuz.interfaces.binarypackagerelease import (
-    BinaryPackageFileType, BinaryPackageFormat)
 from lp.soyuz.interfaces.component import IComponentSet
 from lp.soyuz.interfaces.processor import IProcessorFamilySet
 from lp.soyuz.interfaces.publishing import PackagePublishingStatus
@@ -131,45 +129,11 @@
 
     layer = LaunchpadZopelessLayer
 
-    def publishDDEBInArchive(self, archive):
-        """Publish an arbitrary DDEB with content in to the archive.
-
-        Publishes a DDEB that will take up some space in to `archive`.
-
-        :param archive: the IArchive to publish to.
-        """
-        binarypackagerelease = self.factory.makeBinaryPackageRelease(
-            binpackageformat=BinaryPackageFormat.DDEB)
-        self.factory.makeBinaryPackagePublishingHistory(
-            archive=archive, binarypackagerelease=binarypackagerelease,
-            status=PackagePublishingStatus.PUBLISHED)
-        self.factory.makeBinaryPackageFile(
-            binarypackagerelease=binarypackagerelease,
-            filetype=BinaryPackageFileType.DDEB)
-
     def test_empty_ppa_has_zero_binaries_size(self):
         # An empty PPA has no binaries so has zero binaries_size.
         ppa = self.factory.makeArchive(purpose=ArchivePurpose.PPA)
         self.assertEquals(0, ppa.binaries_size)
 
-    def test_binaries_size_does_not_include_ddebs_for_ppas(self):
-        # DDEBs are not computed in the PPA binaries size because
-        # they are not being published. See bug #399444.
-        ppa = self.factory.makeArchive(purpose=ArchivePurpose.PPA)
-        self.publishDDEBInArchive(ppa)
-        self.assertEquals(0, ppa.binaries_size)
-
-    def test_empty_primary_archive_has_zero_binaries_size(self):
-        # PRIMARY archives have zero binaries_size when created.
-        archive = self.factory.makeArchive(purpose=ArchivePurpose.PRIMARY)
-        self.assertEquals(0, archive.binaries_size)
-
-    def test_binaries_size_includes_ddebs_for_other_archives(self):
-        # DDEBs size are computed for all archive purposes, except PPAs.
-        archive = self.factory.makeArchive(purpose=ArchivePurpose.PRIMARY)
-        self.publishDDEBInArchive(archive)
-        self.assertNotEquals(0, archive.binaries_size)
-
     def test_sources_size_on_empty_archive(self):
         # Zero is returned for an archive without sources.
         archive = self.factory.makeArchive()