Thanks for the review Aaron. On Monday 14 September 2009 18:15:48 Aaron Bentley wrote: > This looks basically good, but there are a few things I'd like changed. > > 1. Please document the parameters of newBinaryPublication and > newSourcePublication Done. > 2. Both methods take a status parameter which they > ignore. Either they should not accept the parameter, or they should not > ignore it. Celso suggests they should not accept the parameter Right, I had intended to take that out and completely forgot, as you can see from the half-assed attempt! Thanks for spotting it, both methods now create new publications with a default status of PENDING. > 3. > xx-queue-pages-delayed-copies.txt appears to reimplement > canonical.launchpad.scripts.tests.run_script. If possible, please use > run_script. > I've changed that. Partial diff below. Cheers Julian === modified file 'lib/lp/soyuz/doc/publishing.txt' --- lib/lp/soyuz/doc/publishing.txt 2009-09-11 10:47:43 +0000 +++ lib/lp/soyuz/doc/publishing.txt 2009-09-15 07:55:17 +0000 @@ -1014,7 +1014,6 @@ ... distroseries=test_source_pub.distroseries, ... component=test_source_pub.component, ... section=test_source_pub.section, - ... status=test_source_pub.status, ... pocket=test_source_pub.pocket) >>> print ppa_pub.component.name main @@ -1029,7 +1028,6 @@ ... component=test_bin_pub.component, ... section=test_bin_pub.section, ... priority=test_bin_pub.priority, - ... status=test_bin_pub.status, ... pocket=test_bin_pub.pocket) >>> print ppa_pub.binarypackagerelease.component.name universe === modified file 'lib/lp/soyuz/interfaces/publishing.py' --- lib/lp/soyuz/interfaces/publishing.py 2009-09-10 15:25:45 +0000 +++ lib/lp/soyuz/interfaces/publishing.py 2009-09-15 09:10:22 +0000 @@ -847,17 +847,34 @@ """Auxiliary methods for dealing with sets of publications.""" def newBinaryPublication(archive, binarypackagerelease, distroarchseries, - component, section, priority, status, pocket): + component, section, priority, pocket): """Create a new `BinaryPackagePublishingHistory`. + :param archive: An `IArchive` + :param binarypackagerelease: An `IBinaryPackageRelease` + :param distroarchseries: An `IDistroArchSeries` + :param component: An `IComponent` + :param section: An `ISection` + :param priority: A `PackagePublishingPriority` + :param pocket: A `PackagePublishingPocket` + datecreated will be UTC_NOW. + status will be PackagePublishingStatus.PENDING """ def newSourcePublication(archive, sourcepackagerelease, distroseries, - component, section, status, pocket): + component, section, pocket): """Create a new `SourcePackagePublishingHistory`. + :param archive: An `IArchive` + :param sourcepackagerelease: An `ISourcePackageRelease` + :param distroseries: An `IDistroSeries` + :param component: An `IComponent` + :param section: An `ISection` + :param pocket: A `PackagePublishingPocket` + datecreated will be UTC_NOW. + status will be PackagePublishingStatus.PENDING """ def getByIdAndArchive(id, archive, source=True): === modified file 'lib/lp/soyuz/model/publishing.py' --- lib/lp/soyuz/model/publishing.py 2009-09-11 10:47:43 +0000 +++ lib/lp/soyuz/model/publishing.py 2009-09-15 07:53:00 +0000 @@ -786,7 +786,6 @@ distroseries, current.component, current.section, - PackagePublishingStatus.PENDING, pocket ) @@ -1051,7 +1050,6 @@ current.component, current.section, current.priority, - PackagePublishingStatus.PENDING, pocket ) copies.append(copy) @@ -1104,7 +1102,7 @@ def newBinaryPublication(self, archive, binarypackagerelease, distroarchseries, component, section, priority, - status, pocket): + pocket): """See `IPublishingSet`.""" if archive.is_ppa: # PPA component must always be 'main', so we override it @@ -1127,8 +1125,7 @@ return BinaryPackagePublishingHistory.get(pub.id) def newSourcePublication(self, archive, sourcepackagerelease, - distroseries, component, section, status, - pocket): + distroseries, component, section, pocket): """See `IPublishingSet`.""" if archive.is_ppa: # PPA component must always be 'main', so we override it === modified file 'lib/lp/soyuz/model/queue.py' --- lib/lp/soyuz/model/queue.py 2009-09-14 15:19:37 +0000 +++ lib/lp/soyuz/model/queue.py 2009-09-15 07:53:28 +0000 @@ -1335,7 +1335,6 @@ component=binary.component, section=binary.section, priority=binary.priority, - status=PackagePublishingStatus.PENDING, pocket=self.packageupload.pocket ) published_binaries.append(bpph) @@ -1466,7 +1465,6 @@ distroseries=self.packageupload.distroseries, component=self.sourcepackagerelease.component, section=self.sourcepackagerelease.section, - status=PackagePublishingStatus.PENDING, pocket=self.packageupload.pocket ) === modified file 'lib/lp/soyuz/stories/soyuz/xx-queue-pages-delayed- copies.txt' --- lib/lp/soyuz/stories/soyuz/xx-queue-pages-delayed-copies.txt 2009-09-14 15:15:54 +0000 +++ lib/lp/soyuz/stories/soyuz/xx-queue-pages-delayed-copies.txt 2009-09-15 09:19:21 +0000 @@ -107,15 +107,11 @@ # DB objects that have no security adapter are modified during the # delayed copy, the modification must be done in Zopeless mode. >>> import os.path - >>> import subprocess - >>> import sys + >>> from canonical.launchpad.ftests.script import run_script >>> from canonical.config import config >>> script = os.path.join(config.root, "scripts/process-accepted.py") - >>> process = subprocess.Popen( - ... [sys.executable, script, "ubuntu"], - ... stdout=subprocess.PIPE, stderr=subprocess.PIPE) - >>> stdout, stderr = process.communicate() - >>> if process.returncode != 0: + >>> result, stdout, stderr = run_script(script, ["ubuntu"]) + >>> if result != 0: ... print stderr Any user can access the DONE queue and access the delayed-copy