launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #15482
[Merge] lp:~cjwatson/launchpad/queue-copy-archive-links into lp:launchpad
Colin Watson has proposed merging lp:~cjwatson/launchpad/queue-copy-archive-links into lp:launchpad.
Commit message:
Show link to source archive for copies from PPAs in DistroSeries:+queue; add PackageUpload.copy_source_archive property.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
Related bugs:
Bug #1169627 in Launchpad itself: "Show link to source archive for copies in +queue"
https://bugs.launchpad.net/launchpad/+bug/1169627
For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/queue-copy-archive-links/+merge/159250
== Summary ==
When reviewing copies in DistroSeries:+queue, it's a pain to figure out where they came from in order to accurately review the change.
== Proposed fix ==
Linkify the archive displayname, at least for PPAs (archive URLs aren't very useful for primary archives). Add an exported copy_source_archive property to PackageUpload. Between them, these changes will make it easier to track things down by hand and should make it possible to teach our "queue" script to follow the breadcrumb trail automatically.
== LOC Rationale ==
+78. I think this is worth it to eliminate a barrier to review for ~ubuntu-release, particularly in the run-up to 13.04 release; and I should have lots of credit from past changes.
== Tests ==
bin/test -vvct lp.soyuz.browser.tests.test_queue -t lp.soyuz.tests.test_packageupload -t lp.testing.tests.test_factory
== Demo and Q/A ==
Copy a package to a frozen series from (a) Debian and (b) a PPA on dogfood, if there isn't one of each available already. Run the PCJ processor. Check that both render reasonably in the DistroSeries:+queue UI, and that the copy_source_archive property in the API is correct.
--
https://code.launchpad.net/~cjwatson/launchpad/queue-copy-archive-links/+merge/159250
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad/queue-copy-archive-links into lp:launchpad.
=== modified file 'lib/lp/_schema_circular_imports.py'
--- lib/lp/_schema_circular_imports.py 2013-04-03 03:09:04 +0000
+++ lib/lp/_schema_circular_imports.py 2013-04-16 23:19:27 +0000
@@ -591,6 +591,7 @@
IPackageUpload['pocket'].vocabulary = PackagePublishingPocket
patch_reference_property(IPackageUpload, 'distroseries', IDistroSeries)
patch_reference_property(IPackageUpload, 'archive', IArchive)
+patch_reference_property(IPackageUpload, 'copy_source_archive', IArchive)
# IStructuralSubscription
patch_collection_property(
=== modified file 'lib/lp/soyuz/browser/queue.py'
--- lib/lp/soyuz/browser/queue.py 2013-01-08 05:45:26 +0000
+++ lib/lp/soyuz/browser/queue.py 2013-04-16 23:19:27 +0000
@@ -196,11 +196,13 @@
"""Batch-load `PackageCopyJob`s and related information."""
package_copy_jobs = load_related(
PackageCopyJob, uploads, ['package_copy_job_id'])
- load_related(Archive, package_copy_jobs, ['source_archive_id'])
+ archives = load_related(
+ Archive, package_copy_jobs, ['source_archive_id'])
+ person_ids = map(attrgetter('ownerID'), archives)
jobs = load_related(Job, package_copy_jobs, ['job_id'])
- person_ids = map(attrgetter('requester_id'), jobs)
+ person_ids.extend(map(attrgetter('requester_id'), jobs))
list(getUtility(IPersonSet).getPrecachedPersonsFromIDs(
- person_ids, need_validity=True))
+ person_ids, need_validity=True, need_icon=True))
def decoratedQueueBatch(self):
"""Return the current batch, converted to decorated objects.
=== modified file 'lib/lp/soyuz/browser/tests/test_queue.py'
--- lib/lp/soyuz/browser/tests/test_queue.py 2013-01-31 02:02:37 +0000
+++ lib/lp/soyuz/browser/tests/test_queue.py 2013-04-16 23:19:27 +0000
@@ -6,6 +6,7 @@
__metaclass__ = type
from lxml import html
+import soupmatchers
from storm.store import Store
from testtools.matchers import Equals
import transaction
@@ -17,6 +18,7 @@
from lp.archiveuploader.tests import datadir
from lp.registry.interfaces.pocket import PackagePublishingPocket
from lp.services.webapp.escaping import html_escape
+from lp.services.webapp.publisher import canonical_url
from lp.services.webapp.servers import LaunchpadTestRequest
from lp.soyuz.browser.queue import CompletePackageUpload
from lp.soyuz.enums import PackageUploadStatus
@@ -360,8 +362,31 @@
html_text = view()
self.assertIn(upload.package_name, html_text)
# The details section states the sync's origin and requester.
+ source_archive = upload.package_copy_job.source_archive
+ self.assertTextMatchesExpressionIgnoreWhitespace(
+ "Sync from %s," % source_archive.displayname, html_text)
self.assertIn(
- upload.package_copy_job.source_archive.displayname, html_text)
+ upload.package_copy_job.job.requester.displayname, html_text)
+
+ def test_view_renders_copy_upload_from_private_archive(self):
+ login(ADMIN_EMAIL)
+ p3a = self.factory.makeArchive(private=True)
+ upload = self.factory.makeCopyJobPackageUpload(source_archive=p3a)
+ queue_admin = self.factory.makeArchiveAdmin(
+ upload.distroseries.main_archive)
+ with person_logged_in(queue_admin):
+ view = self.makeView(upload.distroseries, queue_admin)
+ html_text = view()
+ self.assertIn(upload.package_name, html_text)
+ # The details section states the sync's origin and requester.
+ source_archive = upload.package_copy_job.source_archive
+ source_archive_url = canonical_url(
+ source_archive, path_only_if_possible=True)
+ self.assertThat(html_text, soupmatchers.HTMLContains(
+ soupmatchers.Tag(
+ "source archive", "a", text=source_archive.displayname,
+ attrs={"href": source_archive_url}),
+ ))
self.assertIn(
upload.package_copy_job.job.requester.displayname, html_text)
@@ -379,6 +404,8 @@
sprs[-1].addFile(dsc)
uploads.append(self.factory.makeCustomPackageUpload(distroseries))
uploads.append(self.factory.makeCopyJobPackageUpload(distroseries))
+ uploads.append(self.factory.makeCopyJobPackageUpload(
+ distroseries, source_archive=self.factory.makeArchive()))
self.factory.makePackageset(
packages=(sprs[0].sourcepackagename, sprs[2].sourcepackagename,
sprs[4].sourcepackagename),
@@ -398,7 +425,7 @@
with StormStatementRecorder() as recorder:
view = self.makeView(distroseries, queue_admin)
view()
- self.assertThat(recorder, HasQueryCount(Equals(54)))
+ self.assertThat(recorder, HasQueryCount(Equals(55)))
class TestCompletePackageUpload(TestCaseWithFactory):
=== modified file 'lib/lp/soyuz/configure.zcml'
--- lib/lp/soyuz/configure.zcml 2013-02-18 03:47:21 +0000
+++ lib/lp/soyuz/configure.zcml 2013-04-16 23:19:27 +0000
@@ -168,6 +168,7 @@
custom_file_urls
customFileUrls
getBinaryProperties
+ copy_source_archive
getFileByName
date_created
sourcepackagerelease
=== modified file 'lib/lp/soyuz/interfaces/queue.py'
--- lib/lp/soyuz/interfaces/queue.py 2013-01-08 01:10:35 +0000
+++ lib/lp/soyuz/interfaces/queue.py 2013-04-16 23:19:27 +0000
@@ -193,6 +193,14 @@
readonly=True),
("devel", dict(exported=False)), exported=True)
+ copy_source_archive = exported(
+ Reference(
+ # Really IArchive, patched in _schema_circular_imports.py
+ schema=Interface,
+ description=_("The archive from which this package was copied, if "
+ "any."),
+ title=_("Copy source archive"), required=False, readonly=True))
+
displayname = exported(
TextLine(
title=_("Generic displayname for a queue item"), readonly=True),
=== modified file 'lib/lp/soyuz/model/queue.py'
--- lib/lp/soyuz/model/queue.py 2013-02-06 08:20:13 +0000
+++ lib/lp/soyuz/model/queue.py 2013-04-16 23:19:27 +0000
@@ -295,6 +295,14 @@
})
return properties
+ @property
+ def copy_source_archive(self):
+ """See `IPackageUpload`."""
+ if self.package_copy_job_id is not None:
+ return self.package_copy_job.source_archive
+ else:
+ return None
+
def getFileByName(self, filename):
"""See `IPackageUpload`."""
if (self.changesfile is not None and
=== modified file 'lib/lp/soyuz/templates/distroseries-queue.pt'
--- lib/lp/soyuz/templates/distroseries-queue.pt 2012-11-15 01:41:14 +0000
+++ lib/lp/soyuz/templates/distroseries-queue.pt 2013-04-16 23:19:27 +0000
@@ -224,15 +224,17 @@
<tr>
<td />
<td tal:condition="view/availableActions" />
- <td colspan="7">
+ <td colspan="7"
+ tal:define="pcj packageupload/package_copy_job">
Sync from
+ <a tal:attributes="href pcj/source_archive/fmt:url"
+ tal:content="pcj/source_archive/displayname"
+ tal:condition="pcj/source_archive/is_ppa" />
<tal:archive
- content="packageupload/package_copy_job/source_archive/displayname">
- Primary Archive for Ubuntu
- </tal:archive>,
+ replace="pcj/source_archive/displayname"
+ condition="not: pcj/source_archive/is_ppa" />,
requested by
- <tal:requester
- content="structure packageupload/package_copy_job/job/requester/fmt:link" />
+ <tal:requester content="structure pcj/job/requester/fmt:link" />
</td>
</tr>
</tal:sync>
=== modified file 'lib/lp/soyuz/tests/test_packageupload.py'
--- lib/lp/soyuz/tests/test_packageupload.py 2013-01-31 02:02:37 +0000
+++ lib/lp/soyuz/tests/test_packageupload.py 2013-04-16 23:19:27 +0000
@@ -902,6 +902,13 @@
transaction.commit()
return upload, self.load(upload, person)
+ def makeCopyJobPackageUpload(self, person, **kwargs):
+ with person_logged_in(person):
+ upload = self.factory.makeCopyJobPackageUpload(
+ distroseries=self.distroseries, **kwargs)
+ transaction.commit()
+ return upload, self.load(upload, person)
+
def makeBinaryPackageUpload(self, person, binarypackagename=None,
component=None):
with person_logged_in(person):
@@ -1293,6 +1300,24 @@
}
self.assertEqual(expected_custom, ws_binaries[-1])
+ def test_copy_info(self):
+ # API clients can inspect properties of copies, including the source
+ # archive.
+ person = self.makeQueueAdmin([self.universe])
+ archive = self.factory.makeArchive()
+ upload, ws_upload = self.makeCopyJobPackageUpload(
+ person, sourcepackagename="hello", source_archive=archive)
+
+ self.assertFalse(ws_upload.contains_source)
+ self.assertFalse(ws_upload.contains_build)
+ self.assertTrue(ws_upload.contains_copy)
+ self.assertEqual("hello", ws_upload.display_name)
+ self.assertEqual("sync", ws_upload.display_arches)
+ self.assertEqual("hello", ws_upload.package_name)
+ with person_logged_in(person):
+ archive_url = api_url(archive)
+ self.assertEndsWith(ws_upload.copy_source_archive_link, archive_url)
+
def test_getPackageUploads_query_count(self):
person = self.makeQueueAdmin([self.universe])
uploads = []
=== modified file 'lib/lp/testing/factory.py'
--- lib/lp/testing/factory.py 2013-03-08 22:17:31 +0000
+++ lib/lp/testing/factory.py 2013-04-16 23:19:27 +0000
@@ -3460,12 +3460,13 @@
return upload
def makeCopyJobPackageUpload(self, distroseries=None,
- sourcepackagename=None, target_pocket=None):
+ sourcepackagename=None, source_archive=None,
+ target_pocket=None):
"""Make a `PackageUpload` with a `PackageCopyJob` attached."""
if distroseries is None:
distroseries = self.makeDistroSeries()
spph = self.makeSourcePackagePublishingHistory(
- sourcepackagename=sourcepackagename)
+ archive=source_archive, sourcepackagename=sourcepackagename)
spr = spph.sourcepackagerelease
job = self.makePlainPackageCopyJob(
package_name=spr.sourcepackagename.name,
=== modified file 'lib/lp/testing/tests/test_factory.py'
--- lib/lp/testing/tests/test_factory.py 2012-09-22 23:58:54 +0000
+++ lib/lp/testing/tests/test_factory.py 2013-04-16 23:19:27 +0000
@@ -1,4 +1,4 @@
-# Copyright 2010-2012 Canonical Ltd. This software is licensed under the
+# Copyright 2010-2013 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
"""Tests for the Launchpad object factory."""
@@ -833,11 +833,14 @@
def test_makeCopyJobPackageUpload_passes_on_args(self):
distroseries = self.factory.makeDistroSeries()
spn = self.factory.makeSourcePackageName()
+ source_archive = self.factory.makeArchive()
pu = self.factory.makeCopyJobPackageUpload(
- distroseries=distroseries, sourcepackagename=spn)
+ distroseries=distroseries, sourcepackagename=spn,
+ source_archive=source_archive)
job = removeSecurityProxy(pu.package_copy_job)
self.assertEqual(distroseries, pu.distroseries)
self.assertEqual(distroseries.distribution, pu.archive.distribution)
+ self.assertEqual(source_archive, job.source_archive)
self.assertEqual(distroseries, job.target_distroseries)
self.assertEqual(spn.name, job.package_name)
