launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

Colin Watson has proposed merging lp:~cjwatson/launchpad/async-copy-unembargo into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #1026976 in Launchpad itself: "Archive:+copy-packages creates job which oopses when async-copying from private to public archive"
  https://bugs.launchpad.net/launchpad/+bug/1026976

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/async-copy-unembargo/+merge/115950

== Summary ==

Bug 1026976: Asynchronous copies via the web UI (available to beta testers) create a PCJ which oopses when trying to copy from a private archive to a public archive.

== Proposed fix ==

Add the unembargo=True flag.  I meant to do this a while back, actually, but I had it in a future cleanup branch so forgot about it.

== Pre-implementation notes ==

Cody Somerville raised a concern that there needs to be some kind of confirmation step here.  While I think this would be reasonable, the synchronous copy path (which is still used for everyone who isn't in launchpad-beta-testers) has no such confirmation step, so there is no reason to block this fix on that.  I've filed that separately as bug 1026979.

== LOC Rationale ==

+106.  This will be comfortably more than offset by removing the synchronous copy path once the asynchronous path has passed its beta testing phase.

== Tests ==

bin/test -vvct archive-views.txt

== Demo and Q/A ==

Upload a package to a private archive on dogfood.  Make sure you're in ~launchpad-beta-testers there, and use +copy-packages to copy it to a public archive.  Run the PCJ; it should complete successfully and make the package's files public.
-- 
https://code.launchpad.net/~cjwatson/launchpad/async-copy-unembargo/+merge/115950
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad/async-copy-unembargo into lp:launchpad.

=== modified file 'lib/lp/soyuz/browser/archive.py'
--- lib/lp/soyuz/browser/archive.py	2012-06-29 08:40:05 +0000
+++ lib/lp/soyuz/browser/archive.py	2012-07-20 11:07:41 +0000
@@ -1322,7 +1322,7 @@
             dest_pocket, include_binaries=include_binaries,
             package_version=spph.sourcepackagerelease.version,
             copy_policy=PackageCopyPolicy.INSECURE,
-            requester=person, sponsored=sponsored)
+            requester=person, sponsored=sponsored, unembargo=True)
 
     return copy_asynchronously_message(len(source_pubs))
 

=== modified file 'lib/lp/soyuz/browser/tests/archive-views.txt'
--- lib/lp/soyuz/browser/tests/archive-views.txt	2011-12-30 06:14:56 +0000
+++ lib/lp/soyuz/browser/tests/archive-views.txt	2012-07-20 11:07:41 +0000
@@ -1,4 +1,6 @@
-= Archive View Classes and Pages =
+==============================
+Archive View Classes and Pages
+==============================
 
 Let's use Celso's PPA for the tests.
 
@@ -6,7 +8,8 @@
     >>> cprov = getUtility(IPersonSet).getByName('cprov')
 
 
-== ArchiveView ==
+ArchiveView
+===========
 
 The ArchiveView includes a few helper methods that make it easier to
 display different types of archives (copy archives, ppas).
@@ -482,7 +485,8 @@
     <p><a rel="nofollow" href="http://example.dom/";>http://...example...
 
 
-== ArchivePackageView ==
+ArchivePackageView
+==================
 
 This view displays detailed information about the archive packages that
 is not so relevant for the PPA index page, such as a summary of build
@@ -525,7 +529,8 @@
     True
 
 
-== ArchivePackageDeletionView ==
+ArchivePackageDeletionView
+==========================
 
 We use ArchivePackageDeletionView to provide the mechnisms used to
 delete packages from a PPA via the UI.
@@ -652,7 +657,8 @@
     2
 
 
-== ArchiveEditDependenciesView ==
+ArchiveEditDependenciesView
+===========================
 
 We use ArchiveEditDependenciesView to provide the mechnisms used to
 add and/or remove archive dependencies for a PPA via the UI.
@@ -1036,7 +1042,8 @@
     >>> cprov_private_ppa.disable()
 
 
-== ArchivePackageCopyingView ==
+ArchivePackageCopyingView
+=========================
 
 This class extends ArchiveSourceSelectionFormView, and thus uses the
 same mechanisms for presenting and filtering available sources for
@@ -1313,7 +1320,8 @@
                        RequiredMissing())
 
 
-=== Copy private files to public archives ===
+Copy private files to public archives
+-------------------------------------
 
 Users are allowed to copy private sources into private PPAs, however
 it happens via 'delayed-copies' not the usual direct copying method.
@@ -1378,8 +1386,106 @@
     >>> print copy.status.name
     ACCEPTED
 
-
-== External dependencies validation ==
+The action may also be performed asynchronously.
+
+    >>> from lp.services.features.testing import FeatureFixture
+    >>> from lp.soyuz.browser.archive import (
+    ...     FEATURE_FLAG_MAX_SYNCHRONOUS_SYNCS,
+    ...     )
+    >>> fixture = FeatureFixture({
+    ...     'soyuz.copypackageppa.enabled': 'on',
+    ...     FEATURE_FLAG_MAX_SYNCHRONOUS_SYNCS: '0',
+    ...     })
+    >>> fixture.setUp()
+
+    >>> login('foo.bar@xxxxxxxxxxxxx')
+    >>> async_private_source = test_publisher.createSource(
+    ...     cprov_private_ppa, 'foocomm', '1.0-1', new_version='3.0-1')
+    >>> transaction.commit()
+
+    >>> print async_private_source.displayname
+    foocomm 3.0-1 in hoary
+
+    >>> login('celso.providelo@xxxxxxxxxxxxx')
+    >>> view = create_initialized_view(
+    ...     cprov_private_ppa, name="+copy-packages",
+    ...     form={
+    ...         'field.selected_sources': [str(async_private_source.id)],
+    ...         'field.destination_archive': 'ubuntu-team/ppa',
+    ...         'field.destination_series': '',
+    ...         'field.include_binaries': 'REBUILD_SOURCES',
+    ...         'field.actions.copy': 'Copy',
+    ...         })
+
+    >>> len(view.errors)
+    0
+
+    >>> for notification in view.request.response.notifications:
+    ...     print extract_text(notification.message)
+    Requested sync of 1 package.
+    Please allow some time for this to be processed.
+
+There is one copy job waiting, which we run.
+
+    >>> from lp.services.config import config
+    >>> from lp.services.job.runner import JobRunner
+    >>> from lp.soyuz.interfaces.packagecopyjob import (
+    ...     IPlainPackageCopyJobSource,
+    ...     )
+    >>> from lp.testing.dbuser import dbuser
+    >>> [job] = getUtility(IPlainPackageCopyJobSource).getActiveJobs(
+    ...     ubuntu_team_ppa)
+    >>> with dbuser(config.IPlainPackageCopyJobSource.dbuser):
+    ...     JobRunner([job]).runAll()
+    >>> print job.status.name
+    COMPLETED
+
+The copy results in a pending source publication.
+
+    >>> [copied_source] = ubuntu_team_ppa.getPublishedSources(
+    ...     name="foocomm", exact_match=True)
+    >>> print copied_source.displayname
+    foocomm 3.0-1 in hoary
+
+If we run the same copy again, it will fail.
+
+    >>> view = create_initialized_view(
+    ...     cprov_private_ppa, name="+copy-packages",
+    ...     form={
+    ...         'field.selected_sources': [str(async_private_source.id)],
+    ...         'field.destination_archive': 'ubuntu-team/ppa',
+    ...         'field.destination_series': '',
+    ...         'field.include_binaries': 'REBUILD_SOURCES',
+    ...         'field.actions.copy': 'Copy',
+    ...         })
+    >>> [job] = getUtility(IPlainPackageCopyJobSource).getActiveJobs(
+    ...     ubuntu_team_ppa)
+    >>> with dbuser(config.IPlainPackageCopyJobSource.dbuser):
+    ...     JobRunner([job]).runAll()
+    >>> print job.status.name
+    FAILED
+
+The job failure is shown in the UI.
+
+    >>> from lp.testing import person_logged_in
+    >>> with person_logged_in(ubuntu_team_ppa.owner):
+    ...     ubuntu_team_ppa_view = create_initialized_view(
+    ...         ubuntu_team_ppa, name="+packages",
+    ...         principal=ubuntu_team_ppa.owner)
+    >>> ubuntu_team_ppa_view.has_pending_copy_jobs is not None
+    True
+    >>> for job in ubuntu_team_ppa_view.package_copy_jobs:
+    ...     print job.status.title, job.package_name, job.package_version
+    ...     print job.error_message
+    Failed foocomm 3.0-1
+    foocomm 3.0-1 in hoary (same version already building in the destination
+    archive for Hoary)
+
+    >>> fixture.cleanUp()
+
+
+External dependencies validation
+================================
 
 The ArchiveAdminView checks the external_dependencies form data to see if
 it's a valid sources.list entry.