launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #08757
[Merge] lp:~cjwatson/launchpad/pocket-permissions into lp:launchpad
Colin Watson has proposed merging lp:~cjwatson/launchpad/pocket-permissions into lp:launchpad.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
Related bugs:
Bug #914779 in Launchpad itself: "Pocket maintainers cannot always upload to their pocket"
https://bugs.launchpad.net/launchpad/+bug/914779
For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/pocket-permissions/+merge/110037
== Summary ==
https://code.launchpad.net/~cjwatson/launchpad/pocket-permissions/+merge/109192 missed a spot, which meant that the new Archive.newPocketUploader method wasn't actually usable.
== Proposed fix ==
Fix the traversal methods, and of course improve test coverage to catch this kind of thing.
== LOC Rationale ==
+86. The previous branch this builds on was -46, and I have 2300 lines of credit so I'd like to count this against some of that.
== Tests ==
bin/test -vvct xx-archive.txt
== Demo and Q/A ==
Same as https://code.launchpad.net/~cjwatson/launchpad/pocket-permissions/+merge/109192.
== Lint ==
./lib/lp/soyuz/stories/webservice/xx-archive.txt
43: want exceeds 78 characters.
47: want exceeds 78 characters.
173: want exceeds 78 characters.
190: want exceeds 78 characters.
207: want exceeds 78 characters.
224: want exceeds 78 characters.
370: want exceeds 78 characters.
431: want exceeds 78 characters.
562: want exceeds 78 characters.
624: want exceeds 78 characters.
Most of this is pre-existing, except for one line which follows the same pattern as the others.
--
https://code.launchpad.net/~cjwatson/launchpad/pocket-permissions/+merge/110037
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad/pocket-permissions into lp:launchpad.
=== modified file 'lib/lp/soyuz/browser/archive.py'
--- lib/lp/soyuz/browser/archive.py 2012-06-12 10:59:43 +0000
+++ lib/lp/soyuz/browser/archive.py 2012-06-13 11:15:29 +0000
@@ -402,6 +402,12 @@
if series is not None:
the_item = getUtility(IPackagesetSet).getByName(
item, distroseries=series)
+ elif item_type == 'pocket':
+ # See if "item" is a pocket name.
+ try:
+ the_item = PackagePublishingPocket.items[item]
+ except KeyError:
+ pass
else:
the_item = None
=== modified file 'lib/lp/soyuz/browser/archivepermission.py'
--- lib/lp/soyuz/browser/archivepermission.py 2012-04-16 23:02:44 +0000
+++ lib/lp/soyuz/browser/archivepermission.py 2012-06-13 11:15:29 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009 Canonical Ltd. This software is licensed under the
+# Copyright 2009-2012 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
"""Browser views for archivepermission."""
@@ -34,7 +34,7 @@
elif self.context.permission == ArchivePermissionType.QUEUE_ADMIN:
perm_type = "+queue-admin"
else:
- raise AssertionError, (
+ raise AssertionError(
"Unknown permission type %s" % self.context.permission)
username = self.context.person.name
@@ -48,8 +48,10 @@
item = ("type=packageset&item=%s&series=%s" %
(self.context.package_set_name,
self.context.distro_series_name))
+ elif self.context.pocket is not None:
+ item = "type=pocket&item=%s" % self.context.pocket.name
else:
- raise AssertionError, (
+ raise AssertionError(
"One of component, sourcepackagename or package set should "
"be set")
=== modified file 'lib/lp/soyuz/stories/webservice/xx-archive.txt'
--- lib/lp/soyuz/stories/webservice/xx-archive.txt 2012-06-12 04:05:44 +0000
+++ lib/lp/soyuz/stories/webservice/xx-archive.txt 2012-06-13 11:15:29 +0000
@@ -242,7 +242,8 @@
... return (entry['permission'],
... entry['person_link'],
... entry['component_name'],
- ... entry['source_package_name']),
+ ... entry['source_package_name'],
+ ... entry['pocket']),
>>> def show_permission_entries(permissions):
... for entry in sorted(permissions['entries'],
@@ -251,10 +252,11 @@
... print entry['person_link']
... print entry['component_name']
... print entry['source_package_name']
+ ... print entry['pocket']
>>> show_permission_entries(permissions)
- Archive Upload Rights ...~ubuntu-team main None
- Archive Upload Rights ...~ubuntu-team universe None
+ Archive Upload Rights ...~ubuntu-team main None None
+ Archive Upload Rights ...~ubuntu-team universe None None
`getUploadersForPackage` returns all the permissions where someone can
upload a particular package.
@@ -266,7 +268,7 @@
... show_permission_entries(permissions)
>>> show_mozilla_permissions()
- Archive Upload Rights ...~carlos None mozilla-firefox
+ Archive Upload Rights ...~carlos None mozilla-firefox None
Passing a bad package name results in an error:
@@ -368,8 +370,8 @@
http://.../ubuntu/+archive/primary/+upload/name12?type=packagename&item=mozilla-firefox
>>> show_mozilla_permissions()
- Archive Upload Rights ...~carlos None mozilla-firefox
- Archive Upload Rights ...~name12 None mozilla-firefox
+ Archive Upload Rights ...~carlos None mozilla-firefox None
+ Archive Upload Rights ...~name12 None mozilla-firefox None
deletePackageUploader() removes that permission:
@@ -383,7 +385,7 @@
And we can see that it's gone:
>>> show_mozilla_permissions()
- Archive Upload Rights ...~carlos None mozilla-firefox
+ Archive Upload Rights ...~carlos None mozilla-firefox None
getUploadersForComponent returns all the permissions where someone can
upload to a particular component:
@@ -395,7 +397,7 @@
... show_permission_entries(permissions)
>>> show_component_permissions("main")
- Archive Upload Rights ...~ubuntu-team main None
+ Archive Upload Rights ...~ubuntu-team main None None
Passing a bad component name results in an error:
@@ -409,8 +411,8 @@
all components.
>>> show_component_permissions()
- Archive Upload Rights ...~ubuntu-team main None
- Archive Upload Rights ...~ubuntu-team universe None
+ Archive Upload Rights ...~ubuntu-team main None None
+ Archive Upload Rights ...~ubuntu-team universe None None
newComponentUploader adds a new permission for a person to upload to a
component.
@@ -429,10 +431,10 @@
http://.../ubuntu/+archive/primary/+upload/name12?type=component&item=restricted
>>> show_component_permissions()
- Archive Upload Rights ...~name12 restricted None
- Archive Upload Rights ...~ubuntu-team main None
- Archive Upload Rights ...~ubuntu-team restricted None
- Archive Upload Rights ...~ubuntu-team universe None
+ Archive Upload Rights ...~name12 restricted None None
+ Archive Upload Rights ...~ubuntu-team main None None
+ Archive Upload Rights ...~ubuntu-team restricted None None
+ Archive Upload Rights ...~ubuntu-team universe None None
We can use ``checkUpload`` to verify that a person can upload a
sourcepackage.
@@ -459,9 +461,9 @@
And we can see that it's gone:
>>> show_component_permissions()
- Archive Upload Rights ...~ubuntu-team main None
- Archive Upload Rights ...~ubuntu-team restricted None
- Archive Upload Rights ...~ubuntu-team universe None
+ Archive Upload Rights ...~ubuntu-team main None None
+ Archive Upload Rights ...~ubuntu-team restricted None None
+ Archive Upload Rights ...~ubuntu-team universe None None
And ``checkUpload`` now also no longer passes:
@@ -525,8 +527,8 @@
... show_permission_entries(permissions)
>>> show_admins_for_component("main")
- Queue Administration Rights ...~name12 main None
- Queue Administration Rights ...~ubuntu-team main None
+ Queue Administration Rights ...~name12 main None None
+ Queue Administration Rights ...~ubuntu-team main None None
getComponentsForQueueAdmin returns all the permissions relating to components
where the user is able to administer distroseries queues.
@@ -538,10 +540,10 @@
... show_permission_entries(permissions)
>>> show_components_for_admin(name12)
- Queue Administration Rights ...~name12 main None
- Queue Administration Rights ...~name12 multiverse None
- Queue Administration Rights ...~name12 restricted None
- Queue Administration Rights ...~name12 universe None
+ Queue Administration Rights ...~name12 main None None
+ Queue Administration Rights ...~name12 multiverse None None
+ Queue Administration Rights ...~name12 restricted None None
+ Queue Administration Rights ...~name12 universe None None
newQueueAdmin adds a new permission for a person to administer distroseries
queues in a particular component.
@@ -560,11 +562,11 @@
http://.../ubuntu/+archive/primary/+queue-admin/name12?type=component&item=partner
>>> show_components_for_admin(name12)
- Queue Administration Rights ...~name12 main None
- Queue Administration Rights ...~name12 multiverse None
- Queue Administration Rights ...~name12 partner None
- Queue Administration Rights ...~name12 restricted None
- Queue Administration Rights ...~name12 universe None
+ Queue Administration Rights ...~name12 main None None
+ Queue Administration Rights ...~name12 multiverse None None
+ Queue Administration Rights ...~name12 partner None None
+ Queue Administration Rights ...~name12 restricted None None
+ Queue Administration Rights ...~name12 universe None None
deleteQueueAdmin removes that permission.
@@ -578,10 +580,86 @@
And we can see that it's gone:
>>> show_components_for_admin(name12)
- Queue Administration Rights ...~name12 main None
- Queue Administration Rights ...~name12 multiverse None
- Queue Administration Rights ...~name12 restricted None
- Queue Administration Rights ...~name12 universe None
+ Queue Administration Rights ...~name12 main None None
+ Queue Administration Rights ...~name12 multiverse None None
+ Queue Administration Rights ...~name12 restricted None None
+ Queue Administration Rights ...~name12 universe None None
+
+getUploadersForPocket returns all the permissions where someone can upload
+to a particular pocket:
+
+ >>> ubuntu_devel = webservice.get(
+ ... '/distros', api_version='devel').jsonBody()['entries'][0]
+
+ >>> def show_pocket_permissions(pocket):
+ ... permissions = user_webservice.named_get(
+ ... ubuntu_devel['main_archive_link'], 'getUploadersForPocket',
+ ... api_version='devel', pocket=pocket).jsonBody()
+ ... show_permission_entries(permissions)
+
+ >>> show_pocket_permissions('Proposed')
+
+Passing a bad pocket name results in an error:
+
+ >>> print cjwatson_webservice.named_get(
+ ... ubuntu_devel['main_archive_link'], 'getUploadersForPocket',
+ ... api_version='devel', pocket='badpocket')
+ HTTP/1.1 400 Bad Request
+ ...
+ pocket: Invalid value "badpocket". Acceptable values are: ...
+
+newPocketUploader adds a new permission for a person to upload to a pocket.
+
+ >>> response = ubuntu_owner_webservice.named_post(
+ ... ubuntu_devel['main_archive_link'], 'newPocketUploader', {},
+ ... api_version='devel', person=name12['self_link'],
+ ... pocket='Proposed')
+ >>> print response
+ HTTP/1.1 201 Created
+ ...
+
+ >>> new_permission = user_webservice.get(
+ ... response.getHeader('Location')).jsonBody()
+ >>> print new_permission['self_link']
+ http://.../ubuntu/+archive/primary/+upload/name12?type=pocket&item=PROPOSED
+
+ >>> show_pocket_permissions('Proposed')
+ Archive Upload Rights ...~name12 None None Proposed
+
+The person named in the permission can upload a package to this pocket.
+
+ >>> grumpy = user_webservice.get("/ubuntu/grumpy").jsonBody()
+ >>> response = user_webservice.named_get(
+ ... ubuntu['main_archive_link'], 'checkUpload',
+ ... distroseries=grumpy['self_link'],
+ ... sourcepackagename='mozilla-firefox', pocket='Proposed',
+ ... component='restricted', person=name12['self_link'])
+ >>> print(response)
+ HTTP/1.1 200 Ok
+ ...
+
+deletePocketUploader removes that permission:
+
+ >>> print ubuntu_owner_webservice.named_post(
+ ... ubuntu_devel['main_archive_link'], 'deletePocketUploader', {},
+ ... api_version='devel', person=name12['self_link'],
+ ... pocket='Proposed')
+ HTTP/1.1 200 Ok
+ ...
+
+ >>> show_pocket_permissions('Proposed')
+
+ >>> grumpy = user_webservice.get("/ubuntu/grumpy").jsonBody()
+ >>> response = user_webservice.named_get(
+ ... ubuntu['main_archive_link'], 'checkUpload',
+ ... distroseries=grumpy['self_link'],
+ ... sourcepackagename='mozilla-firefox', pocket='Proposed',
+ ... component='restricted', person=name12['self_link'])
+ >>> print(response)
+ HTTP/1.1 403 Forbidden
+ ...
+ The signer of this package has no upload rights to
+ this distribution's primary archive. Did you mean to upload to a PPA?
Malformed archive permission URLs
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Follow ups
