launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #02267
[Merge] lp:~bac/launchpad/bug-693357 into lp:launchpad
Brad Crittenden has proposed merging lp:~bac/launchpad/bug-693357 into lp:launchpad.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
Related bugs:
#693357 has_existing_ppa and has_ppa_with_published_packages should not be on IPerson
https://bugs.launchpad.net/bugs/693357
For more details, see:
https://code.launchpad.net/~bac/launchpad/bug-693357/+merge/45075
= Summary =
has_existing_ppa and has_ppa_with_published_packages should not be
properties on IPerson, they should be on IArchiveSet so
that Soyuz-specific queries are kept in the lp.soyuz domain.
== Proposed fix ==
Remove those methods from IPerson and modify
IArchiveSet.getPPAOwnedByPerson to handle the few call sites that were
using the old methods.
== Pre-implementation notes ==
None
== Implementation details ==
As above.
== Tests ==
bin/test -t archive.txt -t test_peoplemerge -t person-merge.txt
== Demo and Q/A ==
None
= Launchpad lint =
I'll fix the real lint problems.
Checking for conflicts and issues in changed files.
Linting changed files:
lib/lp/soyuz/doc/archive.txt
lib/lp/registry/browser/peoplemerge.py
lib/lp/soyuz/interfaces/archive.py
lib/lp/registry/interfaces/person.py
lib/lp/soyuz/model/archive.py
lib/lp/registry/browser/person.py
lib/lp/registry/model/person.py
./lib/lp/soyuz/doc/archive.txt
894: source exceeds 78 characters.
1368: source has bad indentation.
1668: source exceeds 78 characters.
1673: source exceeds 78 characters.
2113: narrative exceeds 78 characters.
2127: source exceeds 78 characters.
2133: narrative exceeds 78 characters.
2137: source exceeds 78 characters.
2148: narrative exceeds 78 characters.
./lib/lp/soyuz/interfaces/archive.py
568: E301 expected 1 blank line, found 0
696: E301 expected 1 blank line, found 0
719: E301 expected 1 blank line, found 0
742: E301 expected 1 blank line, found 0
842: E301 expected 1 blank line, found 0
./lib/lp/registry/interfaces/person.py
571: E302 expected 2 blank lines, found 1
--
https://code.launchpad.net/~bac/launchpad/bug-693357/+merge/45075
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~bac/launchpad/bug-693357 into lp:launchpad.
=== modified file 'lib/lp/registry/browser/peoplemerge.py'
--- lib/lp/registry/browser/peoplemerge.py 2010-12-17 04:10:23 +0000
+++ lib/lp/registry/browser/peoplemerge.py 2011-01-03 21:26:33 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009 Canonical Ltd. This software is licensed under the
+# Copyright 2009-2011 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
"""People Merge related wiew classes."""
@@ -48,6 +48,8 @@
IRequestPeopleMerge,
)
from lp.services.propertycache import cachedproperty
+from lp.soyuz.enums import ArchiveStatus
+from lp.soyuz.interfaces.archive import IArchiveSet
class RequestPeopleMergeView(LaunchpadFormView):
@@ -134,7 +136,10 @@
mapping=dict(name=dupe_person.name)))
# We cannot merge the teams if there is a PPA with published
# packages on the duplicate person, unless that PPA is removed.
- if dupe_person.has_existing_ppa:
+ dupe_person_ppas = getUtility(IArchiveSet).getPPAOwnedByPerson(
+ dupe_person, statuses=[ArchiveStatus.ACTIVE,
+ ArchiveStatus.DELETING])
+ if dupe_person_ppas is not None:
self.addError(_(
"${name} has a PPA that must be deleted before it "
"can be merged. It may take ten minutes to remove the "
=== modified file 'lib/lp/registry/browser/person.py'
--- lib/lp/registry/browser/person.py 2010-12-20 15:06:51 +0000
+++ lib/lp/registry/browser/person.py 2011-01-03 21:26:33 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2010 Canonical Ltd. This software is licensed under the
+# Copyright 2009-2011 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
# pylint: disable-msg=E0211,E0213,C0322
@@ -327,6 +327,8 @@
from lp.soyuz.browser.archivesubscription import (
traverse_archive_subscription_for_subscriber,
)
+from lp.soyuz.enums import ArchiveStatus
+from lp.soyuz.interfaces.archive import IArchiveSet
from lp.soyuz.interfaces.archivesubscriber import IArchiveSubscriberSet
from lp.soyuz.interfaces.binarypackagebuild import IBinaryPackageBuildSet
from lp.soyuz.interfaces.sourcepackagerelease import ISourcePackageRelease
@@ -4138,7 +4140,10 @@
When a user has a PPA renames are prohibited.
"""
has_ppa_with_published_packages = (
- self.context.has_ppa_with_published_packages)
+ getUtility(IArchiveSet).getPPAOwnedByPerson(
+ self.context, has_packages=True,
+ statuses=[ArchiveStatus.ACTIVE,
+ ArchiveStatus.DELETING]) is not None)
if has_ppa_with_published_packages:
# This makes the field's widget display (i.e. read) only.
self.form_fields['name'].for_display = True
=== modified file 'lib/lp/registry/interfaces/person.py'
--- lib/lp/registry/interfaces/person.py 2010-12-20 15:06:51 +0000
+++ lib/lp/registry/interfaces/person.py 2011-01-03 21:26:33 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2010 Canonical Ltd. This software is licensed under the
+# Copyright 2009-2011 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
# pylint: disable-msg=E0211,E0213
@@ -862,15 +862,6 @@
# Really IArchive, see archive.py
value_type=Reference(schema=Interface)))
- has_existing_ppa = Bool(
- title=_("Does this person have a ppa that is active or not fully "
- "deleted?"),
- readonly=True, required=False)
-
- has_ppa_with_published_packages = Bool(
- title=_("Does this person have a ppa with published packages?"),
- readonly=True, required=False)
-
entitlements = Attribute("List of Entitlements for this person or team.")
structural_subscriptions = Attribute(
@@ -2178,7 +2169,6 @@
"""
-
class IRequestPeopleMerge(Interface):
"""This schema is used only because we want a very specific vocabulary."""
=== modified file 'lib/lp/registry/model/person.py'
--- lib/lp/registry/model/person.py 2010-12-23 11:35:12 +0000
+++ lib/lp/registry/model/person.py 2011-01-03 21:26:33 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2010 Canonical Ltd. This software is licensed under the
+# Copyright 2009-2011 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
# vars() causes W0612
# pylint: disable-msg=E0611,W0212,W0612,C0322
@@ -278,7 +278,6 @@
from lp.soyuz.interfaces.archivepermission import IArchivePermissionSet
from lp.soyuz.interfaces.archivesubscriber import IArchiveSubscriberSet
from lp.soyuz.model.archive import Archive
-from lp.soyuz.model.publishing import SourcePackagePublishingHistory
from lp.soyuz.model.sourcepackagerelease import SourcePackageRelease
from lp.translations.model.hastranslationimports import (
HasTranslationImportsMixin,
@@ -343,6 +342,7 @@
_person_sort_re = re.compile("(?:[^\w\s]|[\d_])", re.U)
+
def person_sort_key(person):
"""Identical to `person_sort_key` in the database."""
# Strip noise out of displayname. We do not have to bother with
@@ -2675,29 +2675,6 @@
return Archive.selectBy(
owner=self, purpose=ArchivePurpose.PPA, orderBy='name')
- @property
- def has_existing_ppa(self):
- """See `IPerson`."""
- result = Store.of(self).find(
- Archive,
- Archive.owner == self.id,
- Archive.purpose == ArchivePurpose.PPA,
- Archive.status.is_in(
- [ArchiveStatus.ACTIVE, ArchiveStatus.DELETING]))
- return not result.is_empty()
-
- @property
- def has_ppa_with_published_packages(self):
- """See `IPerson`."""
- result = Store.of(self).find(
- Archive,
- SourcePackagePublishingHistory.archive == Archive.id,
- Archive.owner == self.id,
- Archive.purpose == ArchivePurpose.PPA,
- Archive.status.is_in(
- [ArchiveStatus.ACTIVE, ArchiveStatus.DELETING]))
- return not result.is_empty()
-
def getPPAByName(self, name):
"""See `IPerson`."""
return getUtility(IArchiveSet).getPPAOwnedByPerson(self, name)
@@ -3798,7 +3775,9 @@
if getUtility(IEmailAddressSet).getByPerson(from_person).count() > 0:
raise AssertionError('from_person still has email addresses.')
- if from_person.has_existing_ppa:
+ if getUtility(IArchiveSet).getPPAOwnedByPerson(
+ from_person, statuses=[ArchiveStatus.ACTIVE,
+ ArchiveStatus.DELETING]) is not None:
raise AssertionError(
'from_person has a ppa in ACTIVE or DELETING status')
=== modified file 'lib/lp/soyuz/doc/archive.txt'
--- lib/lp/soyuz/doc/archive.txt 2010-10-17 09:04:48 +0000
+++ lib/lp/soyuz/doc/archive.txt 2011-01-03 21:26:33 +0000
@@ -1,4 +1,5 @@
-= Personal Package Archives =
+Personal Package Archives
+=========================
An Archive models a Debian Archive, providing operations like
publication lookups and the complete publishing-pipeline from database
@@ -120,7 +121,8 @@
...
Unauthorized: (..., 'buildd_secret', 'launchpad.Commercial')
-Commercial Member, a commercial admin but not an admin, can set 'buildd_secret'.
+Commercial Member, a commercial admin but not an admin, can set
+'buildd_secret'.
>>> login("commercial-member@xxxxxxxxxxxxx")
>>> cprov_private_ppa.buildd_secret = 'not so secret at all'
@@ -282,7 +284,8 @@
False
-== Published Source and Binary Lookup ==
+Published Source and Binary Lookup
+----------------------------------
IArchive implements a published source & binary lookup methods,
returning I{Source, Binary}PackagePublishingHistory objects.
@@ -444,7 +447,8 @@
3
-=== Binary publication lookups ===
+Binary publication lookups
+--------------------------
'getPublishedOnDiskBinaries' returns only unique publications, i.e., it
excludes architecture-independent duplications which is necessary for
@@ -674,7 +678,8 @@
>>> status_lookup.count()
2
-== getBuildCounters ==
+getBuildCounters
+----------------
IArchive.getBuildCounters() allows callsites to quickly present
the number of builds in a pre-defined status for a given IArchive.
@@ -714,7 +719,8 @@
total 17
-== getBuildSummariesForSourceIds() ==
+getBuildSummariesForSourceIds()
+-------------------------------
IArchive.getBuildSummariesForSourceIds() allows callsites to get an update
on the build statuses for a set of source publishing record ids. This
@@ -760,7 +766,8 @@
- i386 build of foobar 1.0 in ubuntu warty RELEASE
-== No public access to IArchiveView methods ==
+No public access to IArchiveView methods
+----------------------------------------
Both the getBuildCounters() and getBuildSummariesForSourceIds() methods are
not publically available, but available only to users who have permission to
@@ -826,7 +833,8 @@
Unauthorized: (..., 'getBuildSummariesForSourceIds', 'launchpad.View')
-== Package Counters ==
+Package Counters
+----------------
IArchive provides properties to calculate the number and the size of
the packages (sources and binaries) currently published in the
@@ -933,7 +941,8 @@
2
-== Getting an Archive's source-package releases ==
+Getting an Archive's source-package releases
+--------------------------------------------
The method getSourcePackageReleases() is provided to return the unique
source-package releases for the archive. By default, all releases will
@@ -952,7 +961,8 @@
For further details see the `TestGetSourcePackageReleases` unit-test.
-== Sources available for deletions ==
+Sources available for deletions
+-------------------------------
'getSourcesForDeletion' is the base for '+delete-packages' page on PPA
context it allows us to lookup for `ISourcePackagePublishingHistory`
@@ -1031,7 +1041,8 @@
>>> flush_database_caches()
-== Build Lookup ==
+Build Lookup
+------------
It also implements a build lookup method, which supports, 'name',
'status' and 'pocket'.
@@ -1087,7 +1098,8 @@
1
-== Archive dependencies ==
+Archive dependencies
+--------------------
An Archive can depend on one or more other archives, such
relationships affects mainly its builds, which will be querying build
@@ -1282,7 +1294,8 @@
...
AssertionError: This dependency does not exist.
-== Creating a package copy request from an IArchive ==
+Creating a package copy request from an IArchive
+------------------------------------------------
The IArchive interface includes a convenience method for creating a
package copy request:
@@ -1318,7 +1331,8 @@
status: NEW
...
-== IArchiveSet Utility ==
+IArchiveSet Utility
+-------------------
This utility provides useful methods to deal with IArchive in other
parts of the system.
@@ -1628,23 +1642,58 @@
>>> jblack_ppas.count()
0
-Another similar method, getPPAOwnedByPersonUser(), will return the named PPA
-owned by the person, or if the person is not supplied will default to the
+Another similar method, getPPAOwnedByPerson(), will return the named PPA
+owned by the person, or if a name is not supplied will default to the
first PPA that the person created.
- >>> print archive_set.getPPAOwnedByPerson(cprov).displayname
- PPA for Celso Providelo
-
- >>> print archive_set.getPPAOwnedByPerson(cprov, name="ppa").displayname
- PPA for Celso Providelo
+ >>> ppa_owner = factory.makePerson(
+ ... name="ppa-owner", displayname="PPA Owner")
+ >>> login_person(ppa_owner)
+
+If no PPAs match the search criteria, and a name is not given, then
+None is returned.
+
+ >>> print archive_set.getPPAOwnedByPerson(ppa_owner)
+ None
+
+ >>> zoobuntu = factory.makeDistribution(name='zoobuntu')
+ >>> ppa1 = factory.makeArchive(
+ ... owner=ppa_owner, name="first-ppa", distribution=zoobuntu)
+ >>> ppa2 = factory.makeArchive(
+ ... owner=ppa_owner, name="second-ppa", distribution=zoobuntu)
+
+ >>> print archive_set.getPPAOwnedByPerson(ppa_owner).displayname
+ PPA named first-ppa for PPA Owner
+
+ >>> print archive_set.getPPAOwnedByPerson(ppa_owner, name="second-ppa").displayname
+ PPA named second-ppa for PPA Owner
If the named PPA does not exist, a NoSuchPPA exception is raised.
- >>> print archive_set.getPPAOwnedByPerson(cprov, name="goat").displayname
+ >>> print archive_set.getPPAOwnedByPerson(ppa_owner, name="goat").displayname
Traceback (most recent call last):
...
NoSuchPPA: No such ppa: 'goat'.
+A list of statuses may be optionally used, which cause the search to
+only select PPAs with one of those statuses. The first PPA will not
+be selected and the second is returned.
+
+ >>> ppa1.status = ArchiveStatus.DELETED
+ >>> ppa2.status = ArchiveStatus.ACTIVE
+ >>> print archive_set.getPPAOwnedByPerson(
+ ... ppa_owner, statuses=[ArchiveStatus.ACTIVE]).displayname
+ PPA named second-ppa for PPA Owner
+
+The parameter has_packages can be specified to filter based on whether
+the PPA has published packages.
+
+ >>> spph = factory.makeSourcePackagePublishingHistory(
+ ... archive=ppa2, status=PackagePublishingStatus.PUBLISHED)
+ >>> print archive_set.getPPAOwnedByPerson(
+ ... ppa_owner, has_packages=True).displayname
+ PPA named second-ppa for PPA Owner
+
The method getPrivatePPAs() will return a result set of all PPAs that are
private.
@@ -1856,7 +1905,8 @@
total 1
-== A general way to get specific archives for a distribution ==
+A general way to get specific archives for a distribution
+---------------------------------------------------------
IArchiveSet also includes the helper method `getArchivesForDistribution`
which can be used to get archives of a specific purpose(s) for a distribution
@@ -1999,7 +2049,7 @@
true-copy copy-owner2 False False
ultimate-copy copy-owner1 False True
-If exclude_disabled is set to True no disabled archives will be
+If exclude_disabled is set to True no disabled archives will be
included:
>>> foobar = getUtility(IPersonSet).getByName('name16')
@@ -2025,8 +2075,8 @@
team-archive t1 True True
ultimate-copy copy-owner1 False True
-A separate argument allows forcing the inclusion of all disabled archives
-the user has access to, so it doesn't include the archive
+A separate argument allows forcing the inclusion of all disabled archives
+the user has access to, so it doesn't include the archive
of juergen that is disabled.
>>> ubuntu_copy_archives = archive_set.getArchivesForDistribution(
@@ -2039,7 +2089,8 @@
ultimate-copy copy-owner1 False True
-== Getting publishing records across a set of Archives ==
+Getting publishing records across a set of Archives
+---------------------------------------------------
The IArchiveSet utility provides a getPublicationsInArchives() method
that can be used to find the current publishing records for a source
@@ -2056,7 +2107,8 @@
... pub.archive.displayname)
mozilla-firefox - 0.9 in Primary Archive for Ubuntu Linux
-== Archive Permission Checking ==
+Archive Permission Checking
+---------------------------
IArchive has two public methods, checkArchivePermission() and canAdministerQueue()
that check a user's permission to upload and/or administer a
@@ -2245,7 +2297,8 @@
True
-== Rebuild archives ==
+Rebuild archives
+----------------
For further information about how ArchiveRebuild works see
archive-rebuilds.txt. Here we will just document why the creation and
@@ -2316,7 +2369,8 @@
test-rebuild-one
-== Synchronising sources from other archives ==
+Synchronising sources from other archives
+-----------------------------------------
IArchive.syncSources is a convenience wrapper around the copying code
in lp.soyuz.scripts.packagecopier. It allows the caller to
@@ -2523,7 +2577,8 @@
python
-== Publish flag ==
+Publish flag
+------------
Every archive has a "publish" flag that governs whether it should be
published or not. Upon creation that flag is false for copy archives but
@@ -2562,7 +2617,8 @@
False
-== The name uniqueness constraints for archives ==
+The name uniqueness constraints for archives
+--------------------------------------------
The names of archives other than PPAs must be unique for a given
distribution. Trying to create an archive with the same name and
@@ -2642,7 +2698,8 @@
distribution.
-== Looking up named PPAs ==
+Looking up named PPAs
+---------------------
Additionally to the locked 'archive' property, `IPerson` also offers
`ppas` property and `getPPAByName` method.
@@ -2674,7 +2731,8 @@
NoSuchPPA: No such ppa: 'not-found'.
-== Editable displayname ==
+Editable displayname
+--------------------
If 'displayname' is omitted on archive created, a default form is
automatically used.
@@ -2706,7 +2764,8 @@
>>> new_ppa.displayname = 'My testing packages for jaunty'
-== Signing-key propagation ==
+Signing-key propagation
+-----------------------
Signing keys are, by default, shared between PPAs owned by the same
user/team.
@@ -2751,7 +2810,8 @@
True
-== Download counts ==
+Download counts
+---------------
Counts of downloads per binary package release, day and country are kept
up to date by a log-processing script. Archives have a method to get the
=== modified file 'lib/lp/soyuz/interfaces/archive.py'
--- lib/lp/soyuz/interfaces/archive.py 2010-12-03 14:32:17 +0000
+++ lib/lp/soyuz/interfaces/archive.py 2011-01-03 21:26:33 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2010 Canonical Ltd. This software is licensed under the
+# Copyright 2009-2011 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
# pylint: disable-msg=E0211,E0213
@@ -1537,13 +1537,17 @@
def __iter__():
"""Iterates over existent archives, including the main_archives."""
- def getPPAOwnedByPerson(person, name=None):
+ def getPPAOwnedByPerson(person, name=None, statuses=None,
+ has_packages=None):
"""Return the named PPA owned by person.
- :param person: An `IPerson`
- :param name: The PPA name required.
+ :param person: An `IPerson`. Required.
+ :param name: The PPA name. Optional.
+ :param statuses: A list of statuses the PPAs must match. Optional.
+ :param has_packages: If True will only select PPAs that have published
+ source packages.
- If the person is not supplied it will default to the
+ If the name is not supplied it will default to the
first PPA that the person created.
:raises NoSuchPPA: if the named PPA does not exist.
=== modified file 'lib/lp/soyuz/model/archive.py'
--- lib/lp/soyuz/model/archive.py 2010-12-14 20:53:23 +0000
+++ lib/lp/soyuz/model/archive.py 2011-01-03 21:26:33 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009 Canonical Ltd. This software is licensed under the
+# Copyright 2009-2011 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
# pylint: disable-msg=E0611,W0212
@@ -1743,7 +1743,7 @@
"""See `IArchive`."""
if self.is_ppa:
return [PackagePublishingPocket.RELEASE]
-
+
# Cast to a list so we don't trip up with the security proxy not
# understandiung EnumItems.
return list(PackagePublishingPocket.items)
@@ -1935,7 +1935,8 @@
return 0
return int(size)
- def getPPAOwnedByPerson(self, person, name=None):
+ def getPPAOwnedByPerson(self, person, name=None, statuses=None,
+ has_packages=None):
"""See `IArchiveSet`."""
# See Person._all_members which also directly queries this.
store = Store.of(person)
@@ -1944,6 +1945,11 @@
Archive.owner == person]
if name is not None:
clause.append(Archive.name == name)
+ if statuses is not None:
+ clause.append(Archive.status.is_in(statuses))
+ if has_packages:
+ clause.append(
+ SourcePackagePublishingHistory.archive == Archive.id)
result = store.find(Archive, *clause).order_by(Archive.id).first()
if name is not None and result is None:
raise NoSuchPPA(name)
