Merge lp:~cjwatson/launchpad/always-copy-packages-asynchronously-2 into lp:launchpad

Proposed by Colin Watson
Status: Merged
Approved by: William Grant
Approved revision: no longer in the source branch.
Merged at revision: 16270
Proposed branch: lp:~cjwatson/launchpad/always-copy-packages-asynchronously-2
Merge into: lp:launchpad
Diff against target: 1049 lines (+210/-422)
8 files modified
lib/lp/registry/browser/distroseries.py (+1/-5)
lib/lp/registry/browser/tests/test_distroseries.py (+27/-12)
lib/lp/services/features/flags.py (+0/-6)
lib/lp/soyuz/browser/archive.py (+39/-123)
lib/lp/soyuz/browser/tests/archive-views.txt (+8/-59)
lib/lp/soyuz/browser/tests/test_package_copying_mixin.py (+1/-151)
lib/lp/soyuz/model/packagecopyjob.py (+8/-2)
lib/lp/soyuz/stories/ppa/xx-copy-packages.txt (+126/-64)
To merge this branch: bzr merge lp:~cjwatson/launchpad/always-copy-packages-asynchronously-2
Reviewer Review Type Date Requested Status
William Grant code Approve
Review via email: mp+131928@code.launchpad.net

Commit message

Remove the old synchronous package copy path in favour of PackageCopyJobs.

Description of the change

== Summary ==

This is my second attempt at https://code.launchpad.net/~cjwatson/launchpad/always-copy-packages-asynchronously/+merge/131837; Curtis reverted the previous attempt on my behalf since it broke buildbot.

== Proposed fix ==

The complicated bit here was fixing xx-copy-packages.txt, which is a long and horrible doctest built around synchronous copies. The bulk of the fix is fairly mechanical: run copy jobs after each copy operation in the UI. However, to make this work well, I needed to make PCJs say what they're copying in their debug output, as otherwise there was no way to get an accurate idea of which binaries were copied; this seems likely to be occasionally useful elsewhere anyway.

I also noticed that it was a bit odd that the old synchronous path named the target archive in its notification while the new asynchronous path doesn't, and this inconvenienced xx-copy-packages.txt a bit since it wanted to follow that link. I therefore added this to the asynchronous notification.

== Tests ==

bin/test -vvct lp.registry.browser.tests.test_distroseries -t soyuz

== Demo and Q/A ==

Verify that copies between PPAs using the web UI still work.

To post a comment you must log in.
Revision history for this message
Colin Watson (cjwatson) wrote :

Don't review this yet - I made another mistake ...

Revision history for this message
Colin Watson (cjwatson) wrote :

OK, this should be worth reviewing again now.

Revision history for this message
William Grant (wgrant) wrote :

Unless I'm forgetting something, this eliminates the penultimate delayed copy callsite, the last being the API's Archive.syncSource(s). Do you have a strategy for the final excision of that callsite and the significant volume of called code?

240 + if dest_url is None:
241 + dest_url = escape(
242 + canonical_url(dest_archive) + '/+packages', quote=True)
243 +
244 + if dest_display_name is None:
245 + dest_display_name = escape(dest_archive.displayname)

This will probably double escape them; structured() escapes parameters itself. I'd also avoid talking about the escaping in the docstring, as one wouldn't normally expect a string to be passed through unescaped unless the docstring explicitly asked for HTML.

708 + for copy in copied_publications:
709 + self.logger.debug(copy.displayname)

What does the logging around this look like? AFAICT the job doesn't log much else, so it may seem somewhat of a non sequitur in the output.

review: Approve (code)
Revision history for this message
Colin Watson (cjwatson) wrote :

On Tue, Nov 13, 2012 at 05:49:20AM -0000, William Grant wrote:
> Unless I'm forgetting something, this eliminates the penultimate
> delayed copy callsite, the last being the API's Archive.syncSource(s).
> Do you have a strategy for the final excision of that callsite and the
> significant volume of called code?

Not only a strategy but a pending branch that I'll be ready to submit
shortly after this lands.

> This will probably double escape them; structured() escapes parameters
> itself. I'd also avoid talking about the escaping in the docstring, as
> one wouldn't normally expect a string to be passed through unescaped
> unless the docstring explicitly asked for HTML.

This mistake was because compose_synchronous_copy_feedback used
structured() in a slightly different way (with %-formatting rather than
passing separate replacement arguments, so structured() wouldn't have
handled escaping there). Fixed.

> What does the logging around this look like? AFAICT the job doesn't
> log much else, so it may seem somewhat of a non sequitur in the
> output.

I added an extra line of logging to alleviate confusion. (Of course, if
we actually want to see it routinely we'll need to change the PCJ runner
to log at DEBUG; should this be at INFO instead, or is it fine as it
is?)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'lib/lp/registry/browser/distroseries.py'
2--- lib/lp/registry/browser/distroseries.py 2012-07-30 12:11:31 +0000
3+++ lib/lp/registry/browser/distroseries.py 2012-11-13 09:52:27 +0000
4@@ -960,15 +960,11 @@
5
6 sponsored_person = data.get("sponsored_person")
7
8- # When syncing we *must* do it asynchronously so that a package
9- # copy job is created. This gives the job a chance to inspect
10- # the copy and create a PackageUpload if required.
11 if self.do_copy(
12 'selected_differences', sources, self.context.main_archive,
13 self.context, destination_pocket, include_binaries=False,
14 dest_url=series_url, dest_display_name=series_title,
15- person=self.user, force_async=True,
16- sponsored_person=sponsored_person):
17+ person=self.user, sponsored_person=sponsored_person):
18 # The copy worked so we redirect back to show the results. Include
19 # the query string so that the user ends up on the same batch page
20 # with the same filtering parameters as before.
21
22=== modified file 'lib/lp/registry/browser/tests/test_distroseries.py'
23--- lib/lp/registry/browser/tests/test_distroseries.py 2012-09-28 06:25:44 +0000
24+++ lib/lp/registry/browser/tests/test_distroseries.py 2012-11-13 09:52:27 +0000
25@@ -2099,9 +2099,7 @@
26 self.assertEqual(0, len(view.errors))
27 notifications = view.request.response.notifications
28 self.assertEqual(1, len(notifications))
29- self.assertIn(
30- "Requested sync of 1 package.",
31- notifications[0].message)
32+ self.assertIn("Requested sync of 1 package", notifications[0].message)
33 # 302 is a redirect back to the same page.
34 self.assertEqual(302, view.request.response.getStatus())
35
36@@ -2311,25 +2309,42 @@
37 self.assertEqual(1, len(view.cached_differences.batch))
38
39
40-class TestCopyAsynchronouslyMessage(TestCase):
41+class TestCopyAsynchronouslyMessage(TestCaseWithFactory):
42 """Test the helper function `copy_asynchronously_message`."""
43
44+ layer = DatabaseFunctionalLayer
45+
46+ def setUp(self):
47+ super(TestCopyAsynchronouslyMessage, self).setUp()
48+ self.archive = self.factory.makeArchive()
49+ self.series = self.factory.makeDistroSeries()
50+ self.series_url = canonical_url(self.series)
51+ self.series_title = self.series.displayname
52+
53+ def message(self, source_pubs_count):
54+ return copy_asynchronously_message(
55+ source_pubs_count, self.archive, dest_url=self.series_url,
56+ dest_display_name=self.series_title)
57+
58 def test_zero_packages(self):
59 self.assertEqual(
60- "Requested sync of 0 packages.",
61- copy_asynchronously_message(0).escapedtext)
62+ 'Requested sync of 0 packages to <a href="%s">%s</a>.' %
63+ (self.series_url, self.series_title),
64+ self.message(0).escapedtext)
65
66 def test_one_package(self):
67 self.assertEqual(
68- "Requested sync of 1 package.<br />Please "
69- "allow some time for this to be processed.",
70- copy_asynchronously_message(1).escapedtext)
71+ 'Requested sync of 1 package to <a href="%s">%s</a>.<br />'
72+ 'Please allow some time for this to be processed.' %
73+ (self.series_url, self.series_title),
74+ self.message(1).escapedtext)
75
76 def test_multiple_packages(self):
77 self.assertEqual(
78- "Requested sync of 5 packages.<br />Please "
79- "allow some time for these to be processed.",
80- copy_asynchronously_message(5).escapedtext)
81+ 'Requested sync of 5 packages to <a href="%s">%s</a>.<br />'
82+ 'Please allow some time for these to be processed.' %
83+ (self.series_url, self.series_title),
84+ self.message(5).escapedtext)
85
86
87 class TestDistroSeriesNeedsPackagesView(TestCaseWithFactory):
88
89=== modified file 'lib/lp/services/features/flags.py'
90--- lib/lp/services/features/flags.py 2012-11-08 06:06:22 +0000
91+++ lib/lp/services/features/flags.py 2012-11-13 09:52:27 +0000
92@@ -154,12 +154,6 @@
93 '',
94 '',
95 ''),
96- ('soyuz.derived_series.max_synchronous_syncs',
97- 'int',
98- "How many package syncs may be done directly in a web request.",
99- '100',
100- '',
101- ''),
102 ('soyuz.derived_series_sync.enabled',
103 'boolean',
104 'Enables syncing of packages on derivative distributions pages.',
105
106=== modified file 'lib/lp/soyuz/browser/archive.py'
107--- lib/lp/soyuz/browser/archive.py 2012-11-01 03:41:36 +0000
108+++ lib/lp/soyuz/browser/archive.py 2012-11-13 09:52:27 +0000
109@@ -92,11 +92,7 @@
110 get_plural_text,
111 get_user_agent_distroseries,
112 )
113-from lp.services.database.bulk import (
114- load,
115- load_related,
116- )
117-from lp.services.features import getFeatureFlag
118+from lp.services.database.bulk import load_related
119 from lp.services.helpers import english_list
120 from lp.services.job.model.job import Job
121 from lp.services.librarian.browser import FileNavigationMixin
122@@ -166,21 +162,8 @@
123 Archive,
124 validate_ppa,
125 )
126-from lp.soyuz.model.binarypackagename import BinaryPackageName
127-from lp.soyuz.model.publishing import (
128- BinaryPackagePublishingHistory,
129- SourcePackagePublishingHistory,
130- )
131-from lp.soyuz.scripts.packagecopier import (
132- check_copy_permissions,
133- do_copy,
134- )
135-
136-# Feature flag: up to how many package sync requests (inclusive) are to be
137-# processed synchronously within the web request?
138-# Set to -1 to disable synchronous syncs.
139-FEATURE_FLAG_MAX_SYNCHRONOUS_SYNCS = (
140- 'soyuz.derived_series.max_synchronous_syncs')
141+from lp.soyuz.model.publishing import SourcePackagePublishingHistory
142+from lp.soyuz.scripts.packagecopier import check_copy_permissions
143
144
145 class ArchiveBadges(HasBadgeBase):
146@@ -1256,63 +1239,6 @@
147 _messageNoValue = _("vocabulary-copy-to-same-series", "The same series")
148
149
150-def preload_binary_package_names(copies):
151- """Preload `BinaryPackageName`s to speed up display-name construction."""
152- bpn_ids = [
153- copy.binarypackagerelease.binarypackagenameID for copy in copies
154- if isinstance(copy, BinaryPackagePublishingHistory)]
155- load(BinaryPackageName, bpn_ids)
156-
157-
158-def compose_synchronous_copy_feedback(copies, dest_archive, dest_url=None,
159- dest_display_name=None):
160- """Compose human-readable feedback after a synchronous copy."""
161- if dest_url is None:
162- dest_url = escape(
163- canonical_url(dest_archive) + '/+packages', quote=True)
164-
165- if dest_display_name is None:
166- dest_display_name = escape(dest_archive.displayname)
167-
168- if len(copies) == 0:
169- return structured(
170- '<p>All packages already copied to <a href="%s">%s</a>.</p>'
171- % (dest_url, dest_display_name))
172- else:
173- messages = []
174- messages.append(
175- '<p>Packages copied to <a href="%s">%s</a>:</p>'
176- % (dest_url, dest_display_name))
177- messages.append('<ul>')
178- messages.append("\n".join([
179- '<li>%s</li>' % escape(copy) for copy in copies]))
180- messages.append('</ul>')
181- return structured("\n".join(messages))
182-
183-
184-def copy_synchronously(source_pubs, dest_archive, dest_series, dest_pocket,
185- include_binaries, dest_url=None,
186- dest_display_name=None, person=None,
187- check_permissions=True):
188- """Copy packages right now.
189-
190- :return: A `structured` with human-readable feedback about the
191- operation.
192- :raises CannotCopy: If `check_permissions` is True and the copy is
193- not permitted.
194- """
195- copies = do_copy(
196- source_pubs, dest_archive, dest_series, dest_pocket, include_binaries,
197- allow_delayed_copies=True, person=person,
198- check_permissions=check_permissions)
199-
200- preload_binary_package_names(copies)
201-
202- return compose_synchronous_copy_feedback(
203- [copy.displayname for copy in copies], dest_archive, dest_url,
204- dest_display_name)
205-
206-
207 def copy_asynchronously(source_pubs, dest_archive, dest_series, dest_pocket,
208 include_binaries, dest_url=None,
209 dest_display_name=None, person=None,
210@@ -1336,29 +1262,45 @@
211 dest_pocket, include_binaries=include_binaries,
212 package_version=spph.sourcepackagerelease.version,
213 copy_policy=PackageCopyPolicy.INSECURE,
214- requester=person, sponsored=sponsored, unembargo=True)
215-
216- return copy_asynchronously_message(len(source_pubs))
217-
218-
219-def copy_asynchronously_message(source_pubs_count):
220+ requester=person, sponsored=sponsored, unembargo=True,
221+ source_distroseries=spph.distroseries, source_pocket=spph.pocket)
222+
223+ return copy_asynchronously_message(
224+ len(source_pubs), dest_archive, dest_url, dest_display_name)
225+
226+
227+def copy_asynchronously_message(source_pubs_count, dest_archive, dest_url=None,
228+ dest_display_name=None):
229 """Return a message detailing the sync action.
230
231 :param source_pubs_count: The number of source pubs requested for syncing.
232+ :param dest_archive: The destination IArchive.
233+ :param dest_url: The URL of the destination to display in the
234+ notification box. Defaults to the target archive.
235+ :param dest_display_name: The text to use for the dest_url link.
236+ Defaults to the target archive's display name.
237 """
238+ if dest_url is None:
239+ dest_url = canonical_url(dest_archive) + '/+packages'
240+
241+ if dest_display_name is None:
242+ dest_display_name = dest_archive.displayname
243+
244 package_or_packages = get_plural_text(
245 source_pubs_count, "package", "packages")
246 if source_pubs_count == 0:
247 return structured(
248- "Requested sync of %s %s.",
249- source_pubs_count, package_or_packages)
250+ 'Requested sync of %s %s to <a href="%s">%s</a>.',
251+ source_pubs_count, package_or_packages, dest_url,
252+ dest_display_name)
253 else:
254 this_or_these = get_plural_text(
255 source_pubs_count, "this", "these")
256 return structured(
257- "Requested sync of %s %s.<br />"
258+ 'Requested sync of %s %s to <a href="%s">%s</a>.<br />'
259 "Please allow some time for %s to be processed.",
260- source_pubs_count, package_or_packages, this_or_these)
261+ source_pubs_count, package_or_packages, dest_url,
262+ dest_display_name, this_or_these)
263
264
265 def render_cannotcopy_as_html(cannotcopy_exception):
266@@ -1385,28 +1327,14 @@
267 class PackageCopyingMixin:
268 """A mixin class that adds helpers for package copying."""
269
270- def canCopySynchronously(self, source_pubs):
271- """Can we afford to copy `source_pubs` synchronously?"""
272- # Fixed estimate: up to 100 packages can be copied in acceptable
273- # time. Anything more than that and we go async.
274- limit = getFeatureFlag(FEATURE_FLAG_MAX_SYNCHRONOUS_SYNCS)
275- try:
276- limit = int(limit)
277- except:
278- limit = 100
279-
280- return len(source_pubs) <= limit
281-
282 def do_copy(self, sources_field_name, source_pubs, dest_archive,
283 dest_series, dest_pocket, include_binaries,
284 dest_url=None, dest_display_name=None, person=None,
285- check_permissions=True, force_async=False,
286- sponsored_person=None):
287+ check_permissions=True, sponsored_person=None):
288 """Copy packages and add appropriate feedback to the browser page.
289
290- This may either copy synchronously, if there are few enough
291- requests to process right now; or asynchronously in which case
292- it will schedule jobs that will be processed by a script.
293+ This will copy asynchronously, scheduling jobs that will be
294+ processed by a script.
295
296 :param sources_field_name: The name of the form field to set errors
297 on when the copy fails
298@@ -1425,30 +1353,18 @@
299 :param person: The person requesting the copy.
300 :param: check_permissions: boolean indicating whether or not the
301 requester's permissions to copy should be checked.
302- :param force_async: Force the copy to create package copy jobs and
303- perform the copy asynchronously.
304 :param sponsored_person: An IPerson representing the person being
305- sponsored (for asynchronous copies only).
306+ sponsored.
307
308 :return: True if the copying worked, False otherwise.
309 """
310- assert force_async or not sponsored_person, (
311- "sponsored must be None for sync copies")
312 try:
313- if (force_async == False and
314- self.canCopySynchronously(source_pubs)):
315- notification = copy_synchronously(
316- source_pubs, dest_archive, dest_series, dest_pocket,
317- include_binaries, dest_url=dest_url,
318- dest_display_name=dest_display_name, person=person,
319- check_permissions=check_permissions)
320- else:
321- notification = copy_asynchronously(
322- source_pubs, dest_archive, dest_series, dest_pocket,
323- include_binaries, dest_url=dest_url,
324- dest_display_name=dest_display_name, person=person,
325- check_permissions=check_permissions,
326- sponsored=sponsored_person)
327+ notification = copy_asynchronously(
328+ source_pubs, dest_archive, dest_series, dest_pocket,
329+ include_binaries, dest_url=dest_url,
330+ dest_display_name=dest_display_name, person=person,
331+ check_permissions=check_permissions,
332+ sponsored=sponsored_person)
333 except CannotCopy as error:
334 self.setFieldError(
335 sources_field_name, render_cannotcopy_as_html(error))
336
337=== modified file 'lib/lp/soyuz/browser/tests/archive-views.txt'
338--- lib/lp/soyuz/browser/tests/archive-views.txt 2012-10-29 16:52:31 +0000
339+++ lib/lp/soyuz/browser/tests/archive-views.txt 2012-11-13 09:52:27 +0000
340@@ -1342,9 +1342,8 @@
341 Copy private files to public archives
342 -------------------------------------
343
344-Users are allowed to copy private sources into private PPAs, however
345-it happens via 'delayed-copies' not the usual direct copying method.
346-See more information in scripts/packagecopier.py
347+Users are allowed to copy private sources into public PPAs.
348+See more information in scripts/packagecopier.py.
349
350 First we will enable Celso's private PPA.
351
352@@ -1386,61 +1385,13 @@
353 >>> len(view.errors)
354 0
355
356-The action is performed as a delayed-copy, and the user is informed of
357+The action is performed as an asynchronous copy, and the user is informed of
358 it via a page notification.
359
360 >>> from lp.testing.pages import extract_text
361 >>> for notification in view.request.response.notifications:
362 ... print extract_text(notification.message)
363- Packages copied to PPA for Ubuntu Team:
364- Delayed copy of foocomm - 2.0-1 (source)
365-
366-The delayed-copy request is waiting to be processed in the ACCEPTED
367-upload queue.
368-
369- >>> from lp.soyuz.interfaces.queue import IPackageUploadSet
370- >>> copy = getUtility(IPackageUploadSet).findSourceUpload(
371- ... 'foocomm', '2.0-1', ubuntu_team_ppa, ubuntu)
372-
373- >>> print copy.status.name
374- ACCEPTED
375-
376-The action may also be performed asynchronously.
377-
378- >>> from lp.services.features.testing import FeatureFixture
379- >>> from lp.soyuz.browser.archive import (
380- ... FEATURE_FLAG_MAX_SYNCHRONOUS_SYNCS,
381- ... )
382- >>> fixture = FeatureFixture({
383- ... FEATURE_FLAG_MAX_SYNCHRONOUS_SYNCS: '0',
384- ... })
385- >>> fixture.setUp()
386-
387- >>> login('foo.bar@canonical.com')
388- >>> async_private_source = test_publisher.createSource(
389- ... cprov_private_ppa, 'foocomm', '1.0-1', new_version='3.0-1')
390- >>> transaction.commit()
391-
392- >>> print async_private_source.displayname
393- foocomm 3.0-1 in hoary
394-
395- >>> login('celso.providelo@canonical.com')
396- >>> view = create_initialized_view(
397- ... cprov_private_ppa, name="+copy-packages",
398- ... form={
399- ... 'field.selected_sources': [str(async_private_source.id)],
400- ... 'field.destination_archive': 'ubuntu-team/ppa',
401- ... 'field.destination_series': '',
402- ... 'field.include_binaries': 'REBUILD_SOURCES',
403- ... 'field.actions.copy': 'Copy',
404- ... })
405-
406- >>> len(view.errors)
407- 0
408-
409- >>> for notification in view.request.response.notifications:
410- ... print extract_text(notification.message)
411- Requested sync of 1 package.
412+ Requested sync of 1 package to PPA for Ubuntu Team.
413 Please allow some time for this to be processed.
414
415 There is one copy job waiting, which we run.
416@@ -1463,14 +1414,14 @@
417 >>> [copied_source] = ubuntu_team_ppa.getPublishedSources(
418 ... name="foocomm", exact_match=True)
419 >>> print copied_source.displayname
420- foocomm 3.0-1 in hoary
421+ foocomm 2.0-1 in hoary
422
423 If we run the same copy again, it will fail.
424
425 >>> view = create_initialized_view(
426 ... cprov_private_ppa, name="+copy-packages",
427 ... form={
428- ... 'field.selected_sources': [str(async_private_source.id)],
429+ ... 'field.selected_sources': [str(private_source.id)],
430 ... 'field.destination_archive': 'ubuntu-team/ppa',
431 ... 'field.destination_series': '',
432 ... 'field.include_binaries': 'REBUILD_SOURCES',
433@@ -1495,12 +1446,10 @@
434 >>> for job in ubuntu_team_ppa_view.package_copy_jobs:
435 ... print job.status.title, job.package_name, job.package_version
436 ... print job.error_message
437- Failed foocomm 3.0-1
438- foocomm 3.0-1 in hoary (same version already building in the destination
439+ Failed foocomm 2.0-1
440+ foocomm 2.0-1 in hoary (same version already building in the destination
441 archive for Hoary)
442
443- >>> fixture.cleanUp()
444-
445
446 External dependencies validation
447 ================================
448
449=== modified file 'lib/lp/soyuz/browser/tests/test_package_copying_mixin.py'
450--- lib/lp/soyuz/browser/tests/test_package_copying_mixin.py 2012-10-29 16:52:31 +0000
451+++ lib/lp/soyuz/browser/tests/test_package_copying_mixin.py 2012-11-13 09:52:27 +0000
452@@ -1,4 +1,4 @@
453-# Copyright 2011 Canonical Ltd. This software is licensed under the
454+# Copyright 2011-2012 Canonical Ltd. This software is licensed under the
455 # GNU Affero General Public License version 3 (see the file LICENSE).
456
457 """Tests for `PackageCopyingMixin`."""
458@@ -9,14 +9,9 @@
459 from zope.security.proxy import removeSecurityProxy
460
461 from lp.registry.interfaces.pocket import PackagePublishingPocket
462-from lp.services.features.testing import FeatureFixture
463 from lp.services.propertycache import cachedproperty
464 from lp.soyuz.browser.archive import (
465- compose_synchronous_copy_feedback,
466 copy_asynchronously,
467- copy_synchronously,
468- FEATURE_FLAG_MAX_SYNCHRONOUS_SYNCS,
469- PackageCopyingMixin,
470 render_cannotcopy_as_html,
471 )
472 from lp.soyuz.enums import SourcePackageFormat
473@@ -29,9 +24,7 @@
474 TestCase,
475 TestCaseWithFactory,
476 )
477-from lp.testing.fakemethod import FakeMethod
478 from lp.testing.layers import LaunchpadFunctionalLayer
479-from lp.testing.views import create_initialized_view
480
481
482 def find_spph_copy(archive, spph):
483@@ -41,40 +34,6 @@
484 name=spr.sourcepackagename.name, version=spr.version).one()
485
486
487-class FakeDistribution:
488- def __init__(self):
489- self.archive = FakeArchive()
490-
491-
492-class FakeDistroSeries:
493- def __init__(self):
494- self.distribution = FakeDistribution()
495-
496-
497-class FakeSPN:
498- name = "spn-name"
499-
500-
501-class FakeSPR:
502- def __init__(self):
503- self.sourcepackagename = FakeSPN()
504- self.version = "1.0"
505-
506-
507-class FakeArchive:
508- def __init__(self, displayname="archive-name"):
509- self.displayname = displayname
510-
511-
512-class FakeSPPH:
513- def __init__(self, archive=None):
514- if archive is None:
515- archive = FakeArchive()
516- self.sourcepackagerelease = FakeSPR()
517- self.displayname = "spph-displayname"
518- self.archive = archive
519-
520-
521 class TestPackageCopyingMixinLight(TestCase):
522 """Test lightweight functions and methods.
523
524@@ -83,25 +42,11 @@
525
526 unique_number = 1
527
528- def getPocket(self):
529- """Return an arbitrary `PackagePublishingPocket`."""
530- return PackagePublishingPocket.SECURITY
531-
532 def getUniqueString(self):
533 """Return an arbitrary string."""
534 self.unique_number += 1
535 return "string_%d_" % self.unique_number
536
537- def test_canCopySynchronously_allows_small_synchronous_copies(self):
538- # Small numbers of packages can be copied synchronously.
539- packages = [self.getUniqueString() for counter in range(3)]
540- self.assertTrue(PackageCopyingMixin().canCopySynchronously(packages))
541-
542- def test_canCopySynchronously_disallows_large_synchronous_copies(self):
543- # Large numbers of packages must be copied asynchronously.
544- packages = [self.getUniqueString() for counter in range(300)]
545- self.assertFalse(PackageCopyingMixin().canCopySynchronously(packages))
546-
547 def test_render_cannotcopy_as_html_lists_errors(self):
548 # render_cannotcopy_as_html includes a CannotCopy error message
549 # into its HTML notice.
550@@ -116,24 +61,6 @@
551 self.assertNotIn(message, html_text)
552 self.assertIn("x&lt;&gt;y", html_text)
553
554- def test_compose_synchronous_copy_feedback_escapes_archive_name(self):
555- # compose_synchronous_copy_feedback escapes archive displaynames.
556- archive = FakeArchive(displayname="a&b")
557- notice = compose_synchronous_copy_feedback(
558- ["hi"], archive, dest_url="/")
559- html_text = notice.escapedtext
560- self.assertNotIn("a&b", html_text)
561- self.assertIn("a&amp;b", html_text)
562-
563- def test_compose_synchronous_copy_feedback_escapes_package_names(self):
564- # compose_synchronous_copy_feedback escapes package names.
565- archive = FakeArchive()
566- notice = compose_synchronous_copy_feedback(
567- ["x<y"], archive, dest_url="/")
568- html_text = notice.escapedtext
569- self.assertNotIn("x<y", html_text)
570- self.assertIn("x&lt;y", html_text)
571-
572
573 class TestPackageCopyingMixinIntegration(TestCaseWithFactory):
574 """Integration tests for `PackageCopyingMixin`."""
575@@ -180,36 +107,12 @@
576 derived_series, SourcePackageFormat.FORMAT_1_0)
577 return derived_series
578
579- def makeView(self):
580- """Create a `PackageCopyingMixin`-based view."""
581- return create_initialized_view(
582- self.makeDerivedSeries(), "+localpackagediffs")
583-
584 def getUploader(self, archive, spn):
585 """Get person with upload rights for the given package and archive."""
586 uploader = archive.owner
587 removeSecurityProxy(archive).newPackageUploader(uploader, spn)
588 return uploader
589
590- def test_canCopySynchronously_obeys_feature_flag(self):
591- packages = [self.getUniqueString() for counter in range(3)]
592- mixin = PackageCopyingMixin()
593- with FeatureFixture({FEATURE_FLAG_MAX_SYNCHRONOUS_SYNCS: 2}):
594- can_copy_synchronously = mixin.canCopySynchronously(packages)
595- self.assertFalse(can_copy_synchronously)
596-
597- def test_copy_synchronously_copies_packages(self):
598- # copy_synchronously copies packages into the destination
599- # archive.
600- spph = self.makeSPPH()
601- dest_series = self.makeDerivedSeries()
602- archive = dest_series.distribution.main_archive
603- pocket = self.factory.getAnyPocket()
604- copy_synchronously(
605- [spph], archive, dest_series, pocket, include_binaries=False,
606- check_permissions=False)
607- self.assertNotEqual(None, find_spph_copy(archive, spph))
608-
609 def test_copy_asynchronously_does_not_copy_packages(self):
610 # copy_asynchronously does not copy packages into the destination
611 # archive; that happens later, asynchronously.
612@@ -222,19 +125,6 @@
613 check_permissions=False, person=self.factory.makePerson())
614 self.assertEqual(None, find_spph_copy(archive, spph))
615
616- def test_copy_synchronously_lists_packages(self):
617- # copy_synchronously returns feedback that includes the names of
618- # packages it copied.
619- spph = self.makeSPPH()
620- dest_series = self.makeDerivedSeries()
621- pocket = self.factory.getAnyPocket()
622- notice = copy_synchronously(
623- [spph], dest_series.distribution.main_archive, dest_series,
624- pocket, include_binaries=False,
625- check_permissions=False).escapedtext
626- self.assertIn(
627- spph.sourcepackagerelease.sourcepackagename.name, notice)
628-
629 def test_copy_asynchronously_creates_copy_jobs(self):
630 # copy_asynchronously creates PackageCopyJobs.
631 spph = self.makeSPPH()
632@@ -281,46 +171,6 @@
633 [("one", spph_one.distroseries), ("two", spph_two.distroseries)],
634 [(job.package_name, job.target_distroseries) for job in jobs])
635
636- def test_do_copy_goes_async_if_canCopySynchronously_says_so(self):
637- # The view opts for asynchronous copying if canCopySynchronously
638- # returns False. This creates PackageCopyJobs.
639- spph = self.makeSPPH()
640- pocket = self.factory.getAnyPocket()
641- view = self.makeView()
642- dest_series = view.context
643- archive = dest_series.distribution.main_archive
644- view.canCopySynchronously = FakeMethod(result=False)
645- view.do_copy(
646- 'selected_differences', [spph], archive, dest_series, pocket,
647- False, check_permissions=False, person=self.factory.makePerson())
648- jobs = list(getUtility(IPlainPackageCopyJobSource).getActiveJobs(
649- archive))
650- self.assertNotEqual([], jobs)
651-
652- def test_copy_synchronously_may_allow_copy(self):
653- # In a normal working situation, copy_synchronously allows a
654- # copy.
655- spph = self.makeSPPH()
656- pocket = PackagePublishingPocket.RELEASE
657- dest_series = self.makeDerivedSeries()
658- dest_archive = dest_series.main_archive
659- spn = spph.sourcepackagerelease.sourcepackagename
660- notification = copy_synchronously(
661- [spph], dest_archive, dest_series, pocket, False,
662- person=self.getUploader(dest_archive, spn))
663- self.assertIn("Packages copied", notification.escapedtext)
664-
665- def test_copy_synchronously_checks_permissions(self):
666- # Unless told not to, copy_synchronously does a permissions
667- # check.
668- spph = self.makeSPPH()
669- pocket = self.factory.getAnyPocket()
670- dest_series = self.makeDistroSeries()
671- self.assertRaises(
672- CannotCopy,
673- copy_synchronously,
674- [spph], dest_series.main_archive, dest_series, pocket, False)
675-
676 def test_copy_asynchronously_may_allow_copy(self):
677 # In a normal working situation, copy_asynchronously allows a
678 # copy.
679
680=== modified file 'lib/lp/soyuz/model/packagecopyjob.py'
681--- lib/lp/soyuz/model/packagecopyjob.py 2012-10-25 11:02:37 +0000
682+++ lib/lp/soyuz/model/packagecopyjob.py 2012-11-13 09:52:27 +0000
683@@ -187,6 +187,7 @@
684
685 def __init__(self, job):
686 self.context = job
687+ self.logger = logging.getLogger()
688
689 @classmethod
690 def get(cls, job_id):
691@@ -574,8 +575,7 @@
692 # Remember the target archive purpose, as otherwise aborting the
693 # transaction will forget it.
694 target_archive_purpose = self.target_archive.purpose
695- logger = logging.getLogger()
696- logger.info("Job:\n%s\nraised CannotCopy:\n%s" % (self, e))
697+ self.logger.info("Job:\n%s\nraised CannotCopy:\n%s" % (self, e))
698 self.abort() # Abort the txn.
699 self.reportFailure(unicode(e))
700
701@@ -671,6 +671,12 @@
702 # does exist we need to make sure it gets moved to DONE.
703 pu.setDone()
704
705+ if copied_publications:
706+ self.logger.debug(
707+ "Packages copied to %s:" % self.target_archive.displayname)
708+ for copy in copied_publications:
709+ self.logger.debug(copy.displayname)
710+
711 def abort(self):
712 """Abort work."""
713 transaction.abort()
714
715=== modified file 'lib/lp/soyuz/stories/ppa/xx-copy-packages.txt'
716--- lib/lp/soyuz/stories/ppa/xx-copy-packages.txt 2012-09-27 02:53:00 +0000
717+++ lib/lp/soyuz/stories/ppa/xx-copy-packages.txt 2012-11-13 09:52:27 +0000
718@@ -137,6 +137,28 @@
719 >>> flush_database_updates()
720 >>> logout()
721
722+Copying packages will create jobs. Define a simple doctest-friendly runner.
723+
724+ >>> from zope.security.proxy import removeSecurityProxy
725+ >>> from lp.services.log.logger import FakeLogger
726+ >>> from lp.soyuz.interfaces.packagecopyjob import (
727+ ... IPlainPackageCopyJobSource,
728+ ... )
729+
730+ >>> def run_copy_jobs():
731+ ... login('foo.bar@canonical.com')
732+ ... source = getUtility(IPlainPackageCopyJobSource)
733+ ... for job in removeSecurityProxy(source).iterReady():
734+ ... job.logger = FakeLogger()
735+ ... job.start(manage_transaction=True)
736+ ... try:
737+ ... job.run()
738+ ... except Exception:
739+ ... job.fail(manage_transaction=True)
740+ ... else:
741+ ... job.complete(manage_transaction=True)
742+ ... logout()
743+
744 Let's say James wants to rebuild the Celso's 'pmount' source in his PPA.
745
746 He is a little confused by the number of packages presented by
747@@ -235,8 +257,11 @@
748 >>> messages = get_feedback_messages(jblack_browser.contents)
749 >>> for msg in messages:
750 ... print msg
751- Packages copied to PPA for James Blackwell:
752- pmount 0.1-1 in hoary
753+ Requested sync of 1 package to PPA for James Blackwell.
754+ Please allow some time for this to be processed.
755+ >>> run_copy_jobs()
756+ DEBUG Packages copied to PPA for James Blackwell:
757+ DEBUG pmount 0.1-1 in hoary
758
759 James uses the link in the copy summary to go straight to the target
760 PPA, his own. There he can see the just copied package as PENDING and
761@@ -377,9 +402,10 @@
762 >>> messages = get_feedback_messages(jblack_browser.contents)
763 >>> for msg in messages:
764 ... print msg
765- There is 1 error.
766- The following source cannot be copied:
767- pmount 0.1-1 in hoary
768+ Requested sync of 1 package to PPA for James Blackwell.
769+ Please allow some time for this to be processed.
770+ >>> run_copy_jobs()
771+ INFO ... raised CannotCopy: pmount 0.1-1 in hoary
772 (same version already building in the destination archive for Hoary)
773
774 Now, knowing that pmount can only be copied within the same PPA if the
775@@ -398,8 +424,10 @@
776 >>> messages = get_feedback_messages(jblack_browser.contents)
777 >>> for msg in messages:
778 ... print msg
779- There is 1 error.
780- The following source cannot be copied:
781+ Requested sync of 1 package to PPA for James Blackwell.
782+ Please allow some time for this to be processed.
783+ >>> run_copy_jobs()
784+ INFO ... raised CannotCopy:
785 pmount 0.1-1 in hoary (source has no binaries to be copied)
786
787 We will mark the pmount build completed, to emulate the situation
788@@ -433,9 +461,10 @@
789 >>> messages = get_feedback_messages(jblack_browser.contents)
790 >>> for msg in messages:
791 ... print msg
792- There is 1 error.
793- The following source cannot be copied:
794- pmount 0.1-1 in hoary
795+ Requested sync of 1 package to PPA for James Blackwell.
796+ Please allow some time for this to be processed.
797+ >>> run_copy_jobs()
798+ INFO ... raised CannotCopy: pmount 0.1-1 in hoary
799 (same version has unpublished binaries in the destination
800 archive for Hoary, please wait for them to be published before
801 copying)
802@@ -453,8 +482,10 @@
803 >>> messages = get_feedback_messages(jblack_browser.contents)
804 >>> for msg in messages:
805 ... print msg
806- There is 1 error.
807- The following source cannot be copied:
808+ Requested sync of 1 package to PPA for James Blackwell.
809+ Please allow some time for this to be processed.
810+ >>> run_copy_jobs()
811+ INFO ... raised CannotCopy:
812 pmount 0.1-1 in hoary (source has no binaries to be copied)
813
814 We will build and publish the architecture independent binary for
815@@ -491,9 +522,13 @@
816 >>> messages = get_feedback_messages(jblack_browser.contents)
817 >>> for msg in messages:
818 ... print msg
819- Packages copied to PPA for James Blackwell:
820- pmount 0.1-1 in grumpy
821- pmount-bin 0.1-1 in grumpy i386
822+ Requested sync of 1 package to PPA for James Blackwell.
823+ Please allow some time for this to be processed.
824+ >>> run_copy_jobs()
825+ DEBUG Packages copied to PPA for James Blackwell:
826+ DEBUG pmount 0.1-1 in grumpy
827+ DEBUG pmount-bin 0.1-1 in grumpy i386
828+ >>> jblack_browser.open(jblack_browser.url)
829
830 Note that only the i386 binary got copied to grumpy since it lacks
831 hppa support.
832@@ -519,9 +554,8 @@
833 pmount - 0.1-1 Pending Grumpy Editors
834 pmount - 0.1-1 (Newer...) Pending Hoary Editors
835
836-If James performs exactly the same copy procedure again, a message
837-stating that all packages involved were already copied to the
838-selected destination PPA will be rendered.
839+If James performs exactly the same copy procedure again, no more packages
840+will be copied.
841
842 >>> jblack_browser.getControl(
843 ... name='field.selected_sources').value = [pmount_pub_id]
844@@ -533,7 +567,9 @@
845 >>> messages = get_feedback_messages(jblack_browser.contents)
846 >>> for msg in messages:
847 ... print msg
848- All packages already copied to PPA for James Blackwell.
849+ Requested sync of 1 package to PPA for James Blackwell.
850+ Please allow some time for this to be processed.
851+ >>> run_copy_jobs()
852
853 After some time, James realises that pmount in hoary doesn't make much
854 sense and simply deletes it, so his users won't be bothered by this
855@@ -603,9 +639,10 @@
856 >>> messages = get_feedback_messages(jblack_browser.contents)
857 >>> for msg in messages:
858 ... print msg
859- There is 1 error.
860- The following source cannot be copied:
861- pmount 0.1-1 in hoary
862+ Requested sync of 1 package to PPA for James Blackwell.
863+ Please allow some time for this to be processed.
864+ >>> run_copy_jobs()
865+ INFO ... raised CannotCopy: pmount 0.1-1 in hoary
866 (same version already has published binaries in the destination
867 archive)
868
869@@ -624,13 +661,16 @@
870 >>> messages = get_feedback_messages(jblack_browser.contents)
871 >>> for msg in messages:
872 ... print msg
873- Packages copied to PPA for James Blackwell:
874- pmount 0.1-1 in warty
875- pmount-bin 0.1-1 in warty hppa
876- pmount-bin 0.1-1 in warty i386
877+ Requested sync of 1 package to PPA for James Blackwell.
878+ Please allow some time for this to be processed.
879+ >>> run_copy_jobs()
880+ DEBUG Packages copied to PPA for James Blackwell:
881+ DEBUG pmount 0.1-1 in warty
882+ DEBUG pmount-bin 0.1-1 in warty hppa
883+ DEBUG pmount-bin 0.1-1 in warty i386
884+ >>> jblack_browser.open(jblack_browser.url)
885
886-James sees the just-copied 'pmount' source in warty pending
887-publication.
888+James sees the just-copied 'pmount' source in warty pending publication.
889
890 >>> print_ppa_packages(jblack_browser.contents)
891 Source Published Status Series Section Build
892@@ -717,12 +757,16 @@
893 >>> messages = get_feedback_messages(jblack_browser.contents)
894 >>> for msg in messages:
895 ... print msg
896- Packages copied to PPA for James Blackwell Friends:
897- iceweasel 1.0 in hoary
898- mozilla-firefox 1.0 in hoary i386
899- pmount 0.1-1 in hoary
900- pmount 0.1-1 in hoary hppa
901- pmount 0.1-1 in hoary i386
902+ Requested sync of 2 packages to PPA for James Blackwell Friends.
903+ Please allow some time for these to be processed.
904+ >>> run_copy_jobs()
905+ DEBUG Packages copied to PPA for James Blackwell Friends:
906+ DEBUG iceweasel 1.0 in hoary
907+ DEBUG mozilla-firefox 1.0 in hoary i386
908+ DEBUG Packages copied to PPA for James Blackwell Friends:
909+ DEBUG pmount 0.1-1 in hoary
910+ DEBUG pmount 0.1-1 in hoary hppa
911+ DEBUG pmount 0.1-1 in hoary i386
912
913 So happy-hacking for James Friends, Celso's 'iceweasel' and 'pmount'
914 sources and binaries are copied to their PPA.
915@@ -780,11 +824,12 @@
916 >>> messages = get_feedback_messages(jblack_browser.contents)
917 >>> for msg in messages:
918 ... print msg
919- There is 1 error.
920- The following sources cannot be copied:
921- iceweasel 1.0 in hoary
922+ Requested sync of 2 packages to PPA for James Blackwell Friends.
923+ Please allow some time for these to be processed.
924+ >>> run_copy_jobs()
925+ INFO ... raised CannotCopy: iceweasel 1.0 in hoary
926 (same version already has published binaries in the destination archive)
927- pmount 0.1-1 in hoary
928+ INFO ... raised CannotCopy: pmount 0.1-1 in hoary
929 (same version already has published binaries in the destination archive)
930
931 James goes wild and decided to create a new team PPA for his sandbox
932@@ -850,12 +895,15 @@
933 >>> messages = get_feedback_messages(jblack_browser.contents)
934 >>> for msg in messages:
935 ... print msg
936- There is 1 error.
937- The following sources cannot be copied:
938- pmount 0.1-1 in grumpy
939- (same version already building in the destination archive for Warty)
940- pmount 0.1-1 in hoary
941- (same version already building in the destination archive for Warty)
942+ Requested sync of 3 packages to PPA for James Blackwell Sandbox.
943+ Please allow some time for these to be processed.
944+ >>> run_copy_jobs()
945+ DEBUG Packages copied to PPA for James Blackwell Sandbox:
946+ DEBUG pmount 0.1-1 in warty
947+ INFO ... raised CannotCopy: pmount 0.1-1 in grumpy
948+ (same version already building in the destination archive for Warty)
949+ INFO ... raised CannotCopy: pmount 0.1-1 in hoary
950+ (same version already building in the destination archive for Warty)
951
952 Due to the copy error, nothing was copied to the destination PPA, not
953 even the 'warty' source, which was not denied.
954@@ -901,10 +949,14 @@
955 >>> messages = get_feedback_messages(jblack_browser.contents)
956 >>> for msg in messages:
957 ... print msg
958- Packages copied to PPA for James Blackwell:
959- pmount 0.1-1 in hoary
960- pmount-bin 0.1-1 in hoary hppa
961- pmount-bin 0.1-1 in hoary i386
962+ Requested sync of 1 package to PPA for James Blackwell.
963+ Please allow some time for this to be processed.
964+ >>> run_copy_jobs()
965+ DEBUG Packages copied to PPA for James Blackwell:
966+ DEBUG pmount 0.1-1 in hoary
967+ DEBUG pmount-bin 0.1-1 in hoary hppa
968+ DEBUG pmount-bin 0.1-1 in hoary i386
969+ >>> jblack_browser.open(jblack_browser.url)
970
971 >>> print_ppa_packages(jblack_browser.contents)
972 Source Published Status Series Section Build
973@@ -983,10 +1035,13 @@
974 >>> messages = get_feedback_messages(jblack_browser.contents)
975 >>> for msg in messages:
976 ... print msg
977- Packages copied to PPA for James Blackwell:
978- foo 2.0 in hoary
979- foo-bin 2.0 in hoary hppa
980- foo-bin 2.0 in hoary i386
981+ Requested sync of 1 package to PPA for James Blackwell.
982+ Please allow some time for this to be processed.
983+ >>> run_copy_jobs()
984+ DEBUG Packages copied to PPA for James Blackwell:
985+ DEBUG foo 2.0 in hoary
986+ DEBUG foo-bin 2.0 in hoary hppa
987+ DEBUG foo-bin 2.0 in hoary i386
988
989 James tries to copy some of Celso's packages that are older than
990 the ones in his own PPA. He is not allowed to copy these older
991@@ -1014,9 +1069,10 @@
992 >>> messages = get_feedback_messages(jblack_browser.contents)
993 >>> for msg in messages:
994 ... print msg
995- There is 1 error.
996- The following source cannot be copied:
997- foo 1.1 in hoary
998+ Requested sync of 1 package to PPA for James Blackwell.
999+ Please allow some time for this to be processed.
1000+ >>> run_copy_jobs()
1001+ INFO ... raised CannotCopy: foo 1.1 in hoary
1002 (version older than the foo 2.0 in hoary published in hoary)
1003
1004 However if he copies it to another suite is just works (tm) since PPAs
1005@@ -1032,10 +1088,13 @@
1006 >>> messages = get_feedback_messages(jblack_browser.contents)
1007 >>> for msg in messages:
1008 ... print msg
1009- Packages copied to PPA for James Blackwell:
1010- foo 1.1 in warty
1011- foo-bin 1.1 in warty hppa
1012- foo-bin 1.1 in warty i386
1013+ Requested sync of 1 package to PPA for James Blackwell.
1014+ Please allow some time for this to be processed.
1015+ >>> run_copy_jobs()
1016+ DEBUG Packages copied to PPA for James Blackwell:
1017+ DEBUG foo 1.1 in warty
1018+ DEBUG foo-bin 1.1 in warty hppa
1019+ DEBUG foo-bin 1.1 in warty i386
1020
1021 >>> jblack_browser.open(
1022 ... 'http://launchpad.dev/~jblack/+archive/ppa/+packages')
1023@@ -1073,9 +1132,10 @@
1024 >>> messages = get_feedback_messages(jblack_browser.contents)
1025 >>> for msg in messages:
1026 ... print msg
1027- There is 1 error.
1028- The following source cannot be copied:
1029- foo 1.1 in hoary
1030+ Requested sync of 1 package to PPA for James Blackwell.
1031+ Please allow some time for this to be processed.
1032+ >>> run_copy_jobs()
1033+ INFO ... raised CannotCopy: foo 1.1 in hoary
1034 (a different source with the same version is published in the
1035 destination archive)
1036
1037@@ -1108,8 +1168,10 @@
1038 >>> messages = get_feedback_messages(jblack_browser.contents)
1039 >>> for msg in messages:
1040 ... print msg
1041- There is 1 error.
1042- The following source cannot be copied:
1043+ Requested sync of 1 package to PPA for James Blackwell.
1044+ Please allow some time for this to be processed.
1045+ >>> run_copy_jobs()
1046+ INFO ... raised CannotCopy:
1047 foo 9.9 in hoary (source has no binaries to be copied)
1048
1049 No game, no matter what he tries, James can't break PPAs.