Merge lp:~salgado/launchpad/vostok-upload-policy into lp:launchpad
- vostok-upload-policy
- Merge into devel
Status: | Merged |
---|---|
Approved by: | Michael Nelson |
Approved revision: | no longer in the source branch. |
Merged at revision: | 11450 |
Proposed branch: | lp:~salgado/launchpad/vostok-upload-policy |
Merge into: | lp:launchpad |
Prerequisite: | lp:~salgado/launchpad/remove-security-upload-policy |
Diff against target: |
719 lines (+340/-84) 13 files modified
lib/lp/archiveuploader/nascentupload.py (+6/-31) lib/lp/archiveuploader/tests/__init__.py (+4/-0) lib/lp/archiveuploader/tests/nascentupload-announcements.txt (+2/-1) lib/lp/archiveuploader/tests/nascentupload.txt (+2/-1) lib/lp/archiveuploader/tests/nascentuploadfile.txt (+3/-2) lib/lp/archiveuploader/tests/test_buildduploads.py (+130/-4) lib/lp/archiveuploader/tests/test_nascentupload_documentation.py (+2/-1) lib/lp/archiveuploader/tests/test_ppauploadprocessor.py (+3/-6) lib/lp/archiveuploader/tests/test_uploadpolicy.py (+127/-0) lib/lp/archiveuploader/uploadpolicy.py (+52/-14) lib/lp/code/model/sourcepackagerecipebuild.py (+2/-5) lib/lp/soyuz/browser/tests/distroseriesqueue-views.txt (+2/-1) lib/lp/soyuz/doc/soyuz-set-of-uploads.txt (+5/-18) |
To merge this branch: | bzr merge lp:~salgado/launchpad/vostok-upload-policy |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Jeroen T. Vermeulen (community) | Approve | ||
Review via email: mp+33584@code.launchpad.net |
Commit message
Description of the change
This branch moves the code related to validating the upload type from nascentupload to the upload policy, as a separate method. This makes it easy to unit test that code as well as adding more flexibility for subclasses that might want to validate differently.
It also replaces the three can_upload_* variables with a single enum variable.
Guilherme Salgado (salgado) wrote : | # |
On Wed, 2010-08-25 at 05:27 +0000, Jeroen T. Vermeulen wrote:
> Review: Approve
> Beautiful. A few notes, of course:
>
> Lines 199 and 319 of the diff have multiple imports that don't conform
> to the new import formatting rules:
>
> from foobar import (
> Bar,
> Foo,
> )
Oops, fixed.
>
> In line 379 of the diff, the "else" and the assertion throw the
> symmetry of the code slightly out of whack. Just an idea, but maybe
> you can move it out into a separate "else" clause?
>
> elif upload.binaryful:
> # [...]
> else:
> raise AssertionError(
That wfm.
>
> The list of hand-wrapped text lines at line 382 of the diff puzzles
> me. Why not a single dedent(
That's code I just moved around, but I've changed it as you suggest.
>
> Finally, I think I see either an indentation problem or something
Broken indentation indeed; didn't notice when moving the code around.
> involving tabs around line 390. Did you run "make lint" over the
> current form of this branch?
I never did because 'bzr send' would do that for me, but now that I'm
using bzr lp-submit...
>
> But that's all small fry. The branch is definitely an improvement.
Thanks for the review!
If I can make a suggestion as well, it'd be much better if you could
include chunks of the diff in your review[1], for context, instead of
using the line numbers of the diff. That way we don't have to go back
and forth between your comments and the diff (in my case between my mail
client and the merge-proposal's page), but more importantly because the
diff is a moving target, so there's a big chance that the line numbers
have changed when I get to reply to your review. :)
[1] I do that by replying to the m-p email on my mail client, so the
diff is quoted and I just insert my comments at the appropriate places.
Michael Nelson (michael.nelson) wrote : | # |
r=me for -r11430..11432.
Preview Diff
1 | === modified file 'lib/lp/archiveuploader/nascentupload.py' | |||
2 | --- lib/lp/archiveuploader/nascentupload.py 2010-08-24 08:43:51 +0000 | |||
3 | +++ lib/lp/archiveuploader/nascentupload.py 2010-08-25 17:14:48 +0000 | |||
4 | @@ -146,10 +146,11 @@ | |||
5 | 146 | UploadError will be raised and sent up to the caller. If this happens | 146 | UploadError will be raised and sent up to the caller. If this happens |
6 | 147 | the caller should call the reject method and process a rejection. | 147 | the caller should call the reject method and process a rejection. |
7 | 148 | """ | 148 | """ |
8 | 149 | policy = self.policy | ||
9 | 149 | self.logger.debug("Beginning processing.") | 150 | self.logger.debug("Beginning processing.") |
10 | 150 | 151 | ||
11 | 151 | try: | 152 | try: |
13 | 152 | self.policy.setDistroSeriesAndPocket(self.changes.suite_name) | 153 | policy.setDistroSeriesAndPocket(self.changes.suite_name) |
14 | 153 | except NotFoundError: | 154 | except NotFoundError: |
15 | 154 | self.reject( | 155 | self.reject( |
16 | 155 | "Unable to find distroseries: %s" % self.changes.suite_name) | 156 | "Unable to find distroseries: %s" % self.changes.suite_name) |
17 | @@ -185,36 +186,12 @@ | |||
18 | 185 | isinstance(self.changes.files[0], CustomUploadFile)): | 186 | isinstance(self.changes.files[0], CustomUploadFile)): |
19 | 186 | self.logger.debug("Single Custom Upload detected.") | 187 | self.logger.debug("Single Custom Upload detected.") |
20 | 187 | else: | 188 | else: |
44 | 188 | if self.sourceful and not self.policy.can_upload_source: | 189 | policy.validateUploadType(self) |
45 | 189 | self.reject("Upload is sourceful, but policy refuses " | 190 | |
46 | 190 | "sourceful uploads.") | 191 | if self.sourceful and not self.changes.dsc: |
24 | 191 | |||
25 | 192 | elif self.binaryful and not self.policy.can_upload_binaries: | ||
26 | 193 | messages = [ | ||
27 | 194 | "Upload rejected because it contains binary packages.", | ||
28 | 195 | "Ensure you are using `debuild -S`, or an equivalent", | ||
29 | 196 | "command, to generate only the source package before", | ||
30 | 197 | "re-uploading.", | ||
31 | 198 | ] | ||
32 | 199 | if self.is_ppa: | ||
33 | 200 | messages.append( | ||
34 | 201 | "See https://help.launchpad.net/Packaging/PPA for more " | ||
35 | 202 | "information.") | ||
36 | 203 | self.reject(" ".join(messages)) | ||
37 | 204 | |||
38 | 205 | elif (self.sourceful and self.binaryful and | ||
39 | 206 | not self.policy.can_upload_mixed): | ||
40 | 207 | self.reject("Upload is source/binary but policy refuses " | ||
41 | 208 | "mixed uploads.") | ||
42 | 209 | |||
43 | 210 | elif self.sourceful and not self.changes.dsc: | ||
47 | 211 | self.reject( | 192 | self.reject( |
48 | 212 | "Unable to find the DSC file in the source upload.") | 193 | "Unable to find the DSC file in the source upload.") |
49 | 213 | 194 | ||
50 | 214 | else: | ||
51 | 215 | # Upload content are consistent with the current policy. | ||
52 | 216 | pass | ||
53 | 217 | |||
54 | 218 | # Apply the overrides from the database. This needs to be done | 195 | # Apply the overrides from the database. This needs to be done |
55 | 219 | # before doing component verifications because the component | 196 | # before doing component verifications because the component |
56 | 220 | # actually comes from overrides for packages that are not NEW. | 197 | # actually comes from overrides for packages that are not NEW. |
57 | @@ -227,7 +204,7 @@ | |||
58 | 227 | self.verify_acl() | 204 | self.verify_acl() |
59 | 228 | 205 | ||
60 | 229 | # Perform policy checks. | 206 | # Perform policy checks. |
62 | 230 | self.policy.checkUpload(self) | 207 | policy.checkUpload(self) |
63 | 231 | 208 | ||
64 | 232 | # That's all folks. | 209 | # That's all folks. |
65 | 233 | self.logger.debug("Finished checking upload.") | 210 | self.logger.debug("Finished checking upload.") |
66 | @@ -996,8 +973,6 @@ | |||
67 | 996 | # so late in the game is that in the | 973 | # so late in the game is that in the |
68 | 997 | # mixed-upload case we only have a | 974 | # mixed-upload case we only have a |
69 | 998 | # sourcepackagerelease to verify here! | 975 | # sourcepackagerelease to verify here! |
70 | 999 | assert self.policy.can_upload_mixed, ( | ||
71 | 1000 | "Current policy does not allow mixed uploads.") | ||
72 | 1001 | assert sourcepackagerelease, ( | 976 | assert sourcepackagerelease, ( |
73 | 1002 | "No sourcepackagerelease was found.") | 977 | "No sourcepackagerelease was found.") |
74 | 1003 | binary_package_file.verifySourcePackageRelease( | 978 | binary_package_file.verifySourcePackageRelease( |
75 | 1004 | 979 | ||
76 | === modified file 'lib/lp/archiveuploader/tests/__init__.py' | |||
77 | --- lib/lp/archiveuploader/tests/__init__.py 2010-08-20 20:31:18 +0000 | |||
78 | +++ lib/lp/archiveuploader/tests/__init__.py 2010-08-25 17:14:48 +0000 | |||
79 | @@ -117,6 +117,10 @@ | |||
80 | 117 | # We require the changes to be signed but not the dsc | 117 | # We require the changes to be signed but not the dsc |
81 | 118 | self.unsigned_dsc_ok = True | 118 | self.unsigned_dsc_ok = True |
82 | 119 | 119 | ||
83 | 120 | def validateUploadType(self, upload): | ||
84 | 121 | """We accept uploads of any type.""" | ||
85 | 122 | pass | ||
86 | 123 | |||
87 | 120 | def policySpecificChecks(self, upload): | 124 | def policySpecificChecks(self, upload): |
88 | 121 | """Nothing, let it go.""" | 125 | """Nothing, let it go.""" |
89 | 122 | pass | 126 | pass |
90 | 123 | 127 | ||
91 | === modified file 'lib/lp/archiveuploader/tests/nascentupload-announcements.txt' | |||
92 | --- lib/lp/archiveuploader/tests/nascentupload-announcements.txt 2010-08-25 17:14:46 +0000 | |||
93 | +++ lib/lp/archiveuploader/tests/nascentupload-announcements.txt 2010-08-25 17:14:48 +0000 | |||
94 | @@ -297,9 +297,10 @@ | |||
95 | 297 | NEW binary upload to RELEASE pocket via 'sync' policy (we need to | 297 | NEW binary upload to RELEASE pocket via 'sync' policy (we need to |
96 | 298 | override sync policy to allow binary uploads): | 298 | override sync policy to allow binary uploads): |
97 | 299 | 299 | ||
98 | 300 | >>> from lp.archiveuploader.uploadpolicy import ArchiveUploadType | ||
99 | 300 | >>> modified_sync_policy = getPolicy( | 301 | >>> modified_sync_policy = getPolicy( |
100 | 301 | ... name='sync', distro='ubuntu', distroseries='hoary') | 302 | ... name='sync', distro='ubuntu', distroseries='hoary') |
102 | 302 | >>> modified_sync_policy.can_upload_binaries = True | 303 | >>> modified_sync_policy.accepted_type = ArchiveUploadType.BINARY_ONLY |
103 | 303 | 304 | ||
104 | 304 | >>> bar_src = NascentUpload.from_changesfile_path( | 305 | >>> bar_src = NascentUpload.from_changesfile_path( |
105 | 305 | ... datadir('suite/bar_1.0-1_binary/bar_1.0-1_i386.changes'), | 306 | ... datadir('suite/bar_1.0-1_binary/bar_1.0-1_i386.changes'), |
106 | 306 | 307 | ||
107 | === modified file 'lib/lp/archiveuploader/tests/nascentupload.txt' | |||
108 | --- lib/lp/archiveuploader/tests/nascentupload.txt 2010-08-20 12:11:30 +0000 | |||
109 | +++ lib/lp/archiveuploader/tests/nascentupload.txt 2010-08-25 17:14:48 +0000 | |||
110 | @@ -499,9 +499,10 @@ | |||
111 | 499 | need unsigned changes and binaries uploads (same as security, but also | 499 | need unsigned changes and binaries uploads (same as security, but also |
112 | 500 | accepts non-security uploads) | 500 | accepts non-security uploads) |
113 | 501 | 501 | ||
114 | 502 | >>> from lp.archiveuploader.uploadpolicy import ArchiveUploadType | ||
115 | 502 | >>> modified_sync_policy = getPolicy( | 503 | >>> modified_sync_policy = getPolicy( |
116 | 503 | ... name='sync', distro='ubuntu', distroseries='hoary') | 504 | ... name='sync', distro='ubuntu', distroseries='hoary') |
118 | 504 | >>> modified_sync_policy.can_upload_binaries = True | 505 | >>> modified_sync_policy.accepted_type = ArchiveUploadType.BINARY_ONLY |
119 | 505 | 506 | ||
120 | 506 | >>> ed_bin = NascentUpload.from_changesfile_path( | 507 | >>> ed_bin = NascentUpload.from_changesfile_path( |
121 | 507 | ... datadir('split-upload-test/ed_0.2-20_i386.changes'), | 508 | ... datadir('split-upload-test/ed_0.2-20_i386.changes'), |
122 | 508 | 509 | ||
123 | === modified file 'lib/lp/archiveuploader/tests/nascentuploadfile.txt' | |||
124 | --- lib/lp/archiveuploader/tests/nascentuploadfile.txt 2010-08-20 12:11:30 +0000 | |||
125 | +++ lib/lp/archiveuploader/tests/nascentuploadfile.txt 2010-08-25 17:14:48 +0000 | |||
126 | @@ -531,9 +531,10 @@ | |||
127 | 531 | The timestamp of the files in the .deb are tested against the policy for being | 531 | The timestamp of the files in the .deb are tested against the policy for being |
128 | 532 | too new: | 532 | too new: |
129 | 533 | 533 | ||
130 | 534 | >>> from lp.archiveuploader.uploadpolicy import ArchiveUploadType | ||
131 | 534 | >>> old_only_policy = getPolicy( | 535 | >>> old_only_policy = getPolicy( |
132 | 535 | ... name='insecure', distro='ubuntu', distroseries='hoary') | 536 | ... name='insecure', distro='ubuntu', distroseries='hoary') |
134 | 536 | >>> old_only_policy.can_upload_binaries = True | 537 | >>> old_only_policy.accepted_type = ArchiveUploadType.BINARY_ONLY |
135 | 537 | >>> old_only_policy.future_time_grace = -5 * 365 * 24 * 60 * 60 | 538 | >>> old_only_policy.future_time_grace = -5 * 365 * 24 * 60 * 60 |
136 | 538 | 539 | ||
137 | 539 | >>> ed_binary_deb = DebBinaryUploadFile(ed_deb_path, | 540 | >>> ed_binary_deb = DebBinaryUploadFile(ed_deb_path, |
138 | @@ -549,7 +550,7 @@ | |||
139 | 549 | 550 | ||
140 | 550 | >>> new_only_policy = getPolicy( | 551 | >>> new_only_policy = getPolicy( |
141 | 551 | ... name='insecure', distro='ubuntu', distroseries='hoary') | 552 | ... name='insecure', distro='ubuntu', distroseries='hoary') |
143 | 552 | >>> new_only_policy.can_upload_binaries = True | 553 | >>> new_only_policy.accepted_type = ArchiveUploadType.BINARY_ONLY |
144 | 553 | >>> new_only_policy.earliest_year = 2010 | 554 | >>> new_only_policy.earliest_year = 2010 |
145 | 554 | >>> ed_binary_deb = DebBinaryUploadFile(ed_deb_path, | 555 | >>> ed_binary_deb = DebBinaryUploadFile(ed_deb_path, |
146 | 555 | ... 'e31eeb0b6b3b87e1ea79378df864ffff', | 556 | ... 'e31eeb0b6b3b87e1ea79378df864ffff', |
147 | 556 | 557 | ||
148 | === modified file 'lib/lp/archiveuploader/tests/test_buildduploads.py' | |||
149 | --- lib/lp/archiveuploader/tests/test_buildduploads.py 2010-08-20 20:31:18 +0000 | |||
150 | +++ lib/lp/archiveuploader/tests/test_buildduploads.py 2010-08-25 17:14:48 +0000 | |||
151 | @@ -5,13 +5,139 @@ | |||
152 | 5 | 5 | ||
153 | 6 | __metaclass__ = type | 6 | __metaclass__ = type |
154 | 7 | 7 | ||
155 | 8 | import os | ||
156 | 9 | |||
157 | 10 | from zope.component import getUtility | ||
158 | 11 | |||
159 | 8 | from canonical.database.constants import UTC_NOW | 12 | from canonical.database.constants import UTC_NOW |
160 | 9 | from canonical.launchpad.ftests import import_public_test_keys | 13 | from canonical.launchpad.ftests import import_public_test_keys |
165 | 10 | from canonical.launchpad.interfaces import PackagePublishingStatus | 14 | from canonical.launchpad.interfaces import ( |
166 | 11 | from lp.archiveuploader.tests.test_securityuploads import ( | 15 | PackagePublishingStatus, |
167 | 12 | TestStagedBinaryUploadBase, | 16 | PackageUploadStatus, |
168 | 13 | ) | 17 | ) |
169 | 18 | |||
170 | 19 | from lp.archiveuploader.tests.test_uploadprocessor import ( | ||
171 | 20 | TestUploadProcessorBase, | ||
172 | 21 | ) | ||
173 | 22 | from lp.registry.interfaces.distribution import IDistributionSet | ||
174 | 14 | from lp.registry.interfaces.pocket import PackagePublishingPocket | 23 | from lp.registry.interfaces.pocket import PackagePublishingPocket |
175 | 24 | from lp.soyuz.model.binarypackagebuild import BinaryPackageBuild | ||
176 | 25 | |||
177 | 26 | |||
178 | 27 | class TestStagedBinaryUploadBase(TestUploadProcessorBase): | ||
179 | 28 | name = 'baz' | ||
180 | 29 | version = '1.0-1' | ||
181 | 30 | distribution_name = None | ||
182 | 31 | distroseries_name = None | ||
183 | 32 | pocket = None | ||
184 | 33 | policy = 'buildd' | ||
185 | 34 | no_mails = True | ||
186 | 35 | |||
187 | 36 | @property | ||
188 | 37 | def distribution(self): | ||
189 | 38 | return getUtility(IDistributionSet)[self.distribution_name] | ||
190 | 39 | |||
191 | 40 | @property | ||
192 | 41 | def distroseries(self): | ||
193 | 42 | return self.distribution[self.distroseries_name] | ||
194 | 43 | |||
195 | 44 | @property | ||
196 | 45 | def package_name(self): | ||
197 | 46 | return "%s_%s" % (self.name, self.version) | ||
198 | 47 | |||
199 | 48 | @property | ||
200 | 49 | def source_dir(self): | ||
201 | 50 | return self.package_name | ||
202 | 51 | |||
203 | 52 | @property | ||
204 | 53 | def source_changesfile(self): | ||
205 | 54 | return "%s_source.changes" % self.package_name | ||
206 | 55 | |||
207 | 56 | @property | ||
208 | 57 | def binary_dir(self): | ||
209 | 58 | return "%s_binary" % self.package_name | ||
210 | 59 | |||
211 | 60 | def getBinaryChangesfileFor(self, archtag): | ||
212 | 61 | return "%s_%s.changes" % (self.package_name, archtag) | ||
213 | 62 | |||
214 | 63 | def setUp(self): | ||
215 | 64 | """Setup environment for staged binaries upload via security policy. | ||
216 | 65 | |||
217 | 66 | 1. Setup queue directory and other basic attributes | ||
218 | 67 | 2. Override policy options to get security policy and not send emails | ||
219 | 68 | 3. Setup a common UploadProcessor with the overridden options | ||
220 | 69 | 4. Store number of build present before issuing any upload | ||
221 | 70 | 5. Upload the source package via security policy | ||
222 | 71 | 6. Clean log messages. | ||
223 | 72 | 7. Commit transaction, so the upload source can be seen. | ||
224 | 73 | """ | ||
225 | 74 | super(TestStagedBinaryUploadBase, self).setUp() | ||
226 | 75 | self.options.context = self.policy | ||
227 | 76 | self.options.nomails = self.no_mails | ||
228 | 77 | # Set up the uploadprocessor with appropriate options and logger | ||
229 | 78 | self.uploadprocessor = self.getUploadProcessor(self.layer.txn) | ||
230 | 79 | self.builds_before_upload = BinaryPackageBuild.select().count() | ||
231 | 80 | self.source_queue = None | ||
232 | 81 | self._uploadSource() | ||
233 | 82 | self.log.lines = [] | ||
234 | 83 | self.layer.txn.commit() | ||
235 | 84 | |||
236 | 85 | def assertBuildsCreated(self, amount): | ||
237 | 86 | """Assert that a given 'amount' of build records was created.""" | ||
238 | 87 | builds_count = BinaryPackageBuild.select().count() | ||
239 | 88 | self.assertEqual( | ||
240 | 89 | self.builds_before_upload + amount, builds_count) | ||
241 | 90 | |||
242 | 91 | def _prepareUpload(self, upload_dir): | ||
243 | 92 | """Place a copy of the upload directory into incoming queue.""" | ||
244 | 93 | os.system("cp -a %s %s" % | ||
245 | 94 | (os.path.join(self.test_files_dir, upload_dir), | ||
246 | 95 | os.path.join(self.queue_folder, "incoming"))) | ||
247 | 96 | |||
248 | 97 | def _uploadSource(self): | ||
249 | 98 | """Upload and Accept (if necessary) the base source.""" | ||
250 | 99 | self._prepareUpload(self.source_dir) | ||
251 | 100 | self.uploadprocessor.processChangesFile( | ||
252 | 101 | os.path.join(self.queue_folder, "incoming", self.source_dir), | ||
253 | 102 | self.source_changesfile) | ||
254 | 103 | queue_item = self.uploadprocessor.last_processed_upload.queue_root | ||
255 | 104 | self.assertTrue( | ||
256 | 105 | queue_item is not None, | ||
257 | 106 | "Source Upload Failed\nGot: %s" % "\n".join(self.log.lines)) | ||
258 | 107 | acceptable_statuses = [ | ||
259 | 108 | PackageUploadStatus.NEW, | ||
260 | 109 | PackageUploadStatus.UNAPPROVED, | ||
261 | 110 | ] | ||
262 | 111 | if queue_item.status in acceptable_statuses: | ||
263 | 112 | queue_item.setAccepted() | ||
264 | 113 | # Store source queue item for future use. | ||
265 | 114 | self.source_queue = queue_item | ||
266 | 115 | |||
267 | 116 | def _uploadBinary(self, archtag): | ||
268 | 117 | """Upload the base binary. | ||
269 | 118 | |||
270 | 119 | Ensure it got processed and has a respective queue record. | ||
271 | 120 | Return the IBuild attached to upload. | ||
272 | 121 | """ | ||
273 | 122 | self._prepareUpload(self.binary_dir) | ||
274 | 123 | self.uploadprocessor.processChangesFile( | ||
275 | 124 | os.path.join(self.queue_folder, "incoming", self.binary_dir), | ||
276 | 125 | self.getBinaryChangesfileFor(archtag)) | ||
277 | 126 | queue_item = self.uploadprocessor.last_processed_upload.queue_root | ||
278 | 127 | self.assertTrue( | ||
279 | 128 | queue_item is not None, | ||
280 | 129 | "Binary Upload Failed\nGot: %s" % "\n".join(self.log.lines)) | ||
281 | 130 | self.assertEqual(queue_item.builds.count(), 1) | ||
282 | 131 | return queue_item.builds[0].build | ||
283 | 132 | |||
284 | 133 | def _createBuild(self, archtag): | ||
285 | 134 | """Create a build record attached to the base source.""" | ||
286 | 135 | spr = self.source_queue.sources[0].sourcepackagerelease | ||
287 | 136 | build = spr.createBuild( | ||
288 | 137 | distro_arch_series=self.distroseries[archtag], | ||
289 | 138 | pocket=self.pocket, archive=self.distroseries.main_archive) | ||
290 | 139 | self.layer.txn.commit() | ||
291 | 140 | return build | ||
292 | 15 | 141 | ||
293 | 16 | 142 | ||
294 | 17 | class TestBuilddUploads(TestStagedBinaryUploadBase): | 143 | class TestBuilddUploads(TestStagedBinaryUploadBase): |
295 | 18 | 144 | ||
296 | === modified file 'lib/lp/archiveuploader/tests/test_nascentupload_documentation.py' | |||
297 | --- lib/lp/archiveuploader/tests/test_nascentupload_documentation.py 2010-08-23 13:39:29 +0000 | |||
298 | +++ lib/lp/archiveuploader/tests/test_nascentupload_documentation.py 2010-08-25 17:14:48 +0000 | |||
299 | @@ -30,6 +30,7 @@ | |||
300 | 30 | getPolicy, | 30 | getPolicy, |
301 | 31 | mock_logger_quiet, | 31 | mock_logger_quiet, |
302 | 32 | ) | 32 | ) |
303 | 33 | from lp.archiveuploader.uploadpolicy import ArchiveUploadType | ||
304 | 33 | from lp.registry.interfaces.distribution import IDistributionSet | 34 | from lp.registry.interfaces.distribution import IDistributionSet |
305 | 34 | from lp.soyuz.interfaces.component import IComponentSet | 35 | from lp.soyuz.interfaces.component import IComponentSet |
306 | 35 | 36 | ||
307 | @@ -52,7 +53,7 @@ | |||
308 | 52 | def getUploadForBinary(upload_path): | 53 | def getUploadForBinary(upload_path): |
309 | 53 | """Return a NascentUpload object for binaries.""" | 54 | """Return a NascentUpload object for binaries.""" |
310 | 54 | policy = getPolicy(name='sync', distro='ubuntu', distroseries='hoary') | 55 | policy = getPolicy(name='sync', distro='ubuntu', distroseries='hoary') |
312 | 55 | policy.can_upload_binaries = True | 56 | policy.accepted_type = ArchiveUploadType.BINARY_ONLY |
313 | 56 | return NascentUpload.from_changesfile_path( | 57 | return NascentUpload.from_changesfile_path( |
314 | 57 | datadir(upload_path), policy, mock_logger_quiet) | 58 | datadir(upload_path), policy, mock_logger_quiet) |
315 | 58 | 59 | ||
316 | 59 | 60 | ||
317 | === modified file 'lib/lp/archiveuploader/tests/test_ppauploadprocessor.py' | |||
318 | --- lib/lp/archiveuploader/tests/test_ppauploadprocessor.py 2010-08-20 20:31:18 +0000 | |||
319 | +++ lib/lp/archiveuploader/tests/test_ppauploadprocessor.py 2010-08-25 17:14:48 +0000 | |||
320 | @@ -681,12 +681,9 @@ | |||
321 | 681 | "bar_1.0-1-mixed", "~name16/ubuntu") | 681 | "bar_1.0-1-mixed", "~name16/ubuntu") |
322 | 682 | self.processUpload(self.uploadprocessor, upload_dir) | 682 | self.processUpload(self.uploadprocessor, upload_dir) |
323 | 683 | 683 | ||
330 | 684 | self.assertEqual( | 684 | self.assertIn( |
331 | 685 | self.uploadprocessor.last_processed_upload.rejection_message, | 685 | 'Source/binary (i.e. mixed) uploads are not allowed.', |
332 | 686 | 'Upload rejected because it contains binary packages. Ensure ' | 686 | self.uploadprocessor.last_processed_upload.rejection_message) |
327 | 687 | 'you are using `debuild -S`, or an equivalent command, to ' | ||
328 | 688 | 'generate only the source package before re-uploading. See ' | ||
329 | 689 | 'https://help.launchpad.net/Packaging/PPA for more information.') | ||
333 | 690 | 687 | ||
334 | 691 | def testPGPSignatureNotPreserved(self): | 688 | def testPGPSignatureNotPreserved(self): |
335 | 692 | """PGP signatures should be removed from PPA .changes files. | 689 | """PGP signatures should be removed from PPA .changes files. |
336 | 693 | 690 | ||
337 | === added file 'lib/lp/archiveuploader/tests/test_uploadpolicy.py' | |||
338 | --- lib/lp/archiveuploader/tests/test_uploadpolicy.py 1970-01-01 00:00:00 +0000 | |||
339 | +++ lib/lp/archiveuploader/tests/test_uploadpolicy.py 2010-08-25 17:14:48 +0000 | |||
340 | @@ -0,0 +1,127 @@ | |||
341 | 1 | #!/usr/bin/python | ||
342 | 2 | # | ||
343 | 3 | # Copyright 2010 Canonical Ltd. This software is licensed under the | ||
344 | 4 | # GNU Affero General Public License version 3 (see the file LICENSE). | ||
345 | 5 | |||
346 | 6 | from canonical.testing.layers import DatabaseFunctionalLayer | ||
347 | 7 | |||
348 | 8 | from lp.app.errors import NotFoundError | ||
349 | 9 | from lp.archiveuploader.uploadpolicy import ( | ||
350 | 10 | AbstractUploadPolicy, | ||
351 | 11 | ArchiveUploadType, | ||
352 | 12 | ) | ||
353 | 13 | from lp.testing import TestCase, TestCaseWithFactory | ||
354 | 14 | |||
355 | 15 | |||
356 | 16 | class TestUploadPolicy_validateUploadType(TestCase): | ||
357 | 17 | """Test what kind (sourceful/binaryful/mixed) of uploads are accepted.""" | ||
358 | 18 | |||
359 | 19 | def test_sourceful_accepted(self): | ||
360 | 20 | policy = make_policy(accepted_type=ArchiveUploadType.SOURCE_ONLY) | ||
361 | 21 | upload = make_fake_upload(sourceful=True) | ||
362 | 22 | |||
363 | 23 | policy.validateUploadType(upload) | ||
364 | 24 | |||
365 | 25 | self.assertEquals([], upload.rejections) | ||
366 | 26 | |||
367 | 27 | def test_binaryful_accepted(self): | ||
368 | 28 | policy = make_policy(accepted_type=ArchiveUploadType.BINARY_ONLY) | ||
369 | 29 | upload = make_fake_upload(binaryful=True) | ||
370 | 30 | |||
371 | 31 | policy.validateUploadType(upload) | ||
372 | 32 | |||
373 | 33 | self.assertEquals([], upload.rejections) | ||
374 | 34 | |||
375 | 35 | def test_mixed_accepted(self): | ||
376 | 36 | policy = make_policy(accepted_type=ArchiveUploadType.MIXED_ONLY) | ||
377 | 37 | upload = make_fake_upload(sourceful=True, binaryful=True) | ||
378 | 38 | |||
379 | 39 | policy.validateUploadType(upload) | ||
380 | 40 | |||
381 | 41 | self.assertEquals([], upload.rejections) | ||
382 | 42 | |||
383 | 43 | def test_sourceful_not_accepted(self): | ||
384 | 44 | policy = make_policy(accepted_type=ArchiveUploadType.BINARY_ONLY) | ||
385 | 45 | upload = make_fake_upload(sourceful=True) | ||
386 | 46 | |||
387 | 47 | policy.validateUploadType(upload) | ||
388 | 48 | |||
389 | 49 | self.assertIn( | ||
390 | 50 | 'Sourceful uploads are not accepted by this policy.', | ||
391 | 51 | upload.rejections) | ||
392 | 52 | |||
393 | 53 | def test_binaryful_not_accepted(self): | ||
394 | 54 | policy = make_policy(accepted_type=ArchiveUploadType.SOURCE_ONLY) | ||
395 | 55 | upload = make_fake_upload(binaryful=True) | ||
396 | 56 | |||
397 | 57 | policy.validateUploadType(upload) | ||
398 | 58 | |||
399 | 59 | self.assertTrue(len(upload.rejections) > 0) | ||
400 | 60 | self.assertIn( | ||
401 | 61 | 'Upload rejected because it contains binary packages.', | ||
402 | 62 | upload.rejections[0]) | ||
403 | 63 | |||
404 | 64 | def test_mixed_not_accepted(self): | ||
405 | 65 | policy = make_policy(accepted_type=ArchiveUploadType.SOURCE_ONLY) | ||
406 | 66 | upload = make_fake_upload(sourceful=True, binaryful=True) | ||
407 | 67 | |||
408 | 68 | policy.validateUploadType(upload) | ||
409 | 69 | |||
410 | 70 | self.assertIn( | ||
411 | 71 | 'Source/binary (i.e. mixed) uploads are not allowed.', | ||
412 | 72 | upload.rejections) | ||
413 | 73 | |||
414 | 74 | def test_sourceful_when_only_mixed_accepted(self): | ||
415 | 75 | policy = make_policy(accepted_type=ArchiveUploadType.MIXED_ONLY) | ||
416 | 76 | upload = make_fake_upload(sourceful=True, binaryful=False) | ||
417 | 77 | |||
418 | 78 | policy.validateUploadType(upload) | ||
419 | 79 | |||
420 | 80 | self.assertIn( | ||
421 | 81 | 'Sourceful uploads are not accepted by this policy.', | ||
422 | 82 | upload.rejections) | ||
423 | 83 | |||
424 | 84 | def test_binaryful_when_only_mixed_accepted(self): | ||
425 | 85 | policy = make_policy(accepted_type=ArchiveUploadType.MIXED_ONLY) | ||
426 | 86 | upload = make_fake_upload(sourceful=False, binaryful=True) | ||
427 | 87 | |||
428 | 88 | policy.validateUploadType(upload) | ||
429 | 89 | |||
430 | 90 | self.assertTrue(len(upload.rejections) > 0) | ||
431 | 91 | self.assertIn( | ||
432 | 92 | 'Upload rejected because it contains binary packages.', | ||
433 | 93 | upload.rejections[0]) | ||
434 | 94 | |||
435 | 95 | |||
436 | 96 | class TestUploadPolicy(TestCaseWithFactory): | ||
437 | 97 | |||
438 | 98 | layer = DatabaseFunctionalLayer | ||
439 | 99 | |||
440 | 100 | def test_setDistroSeriesAndPocket_distro_not_found(self): | ||
441 | 101 | policy = AbstractUploadPolicy() | ||
442 | 102 | policy.distro = self.factory.makeDistribution() | ||
443 | 103 | self.assertRaises( | ||
444 | 104 | NotFoundError, policy.setDistroSeriesAndPocket, | ||
445 | 105 | 'nonexistent_security') | ||
446 | 106 | |||
447 | 107 | |||
448 | 108 | class FakeNascentUpload: | ||
449 | 109 | |||
450 | 110 | def __init__(self, sourceful, binaryful): | ||
451 | 111 | self.sourceful = sourceful | ||
452 | 112 | self.binaryful = binaryful | ||
453 | 113 | self.is_ppa = False | ||
454 | 114 | self.rejections = [] | ||
455 | 115 | |||
456 | 116 | def reject(self, msg): | ||
457 | 117 | self.rejections.append(msg) | ||
458 | 118 | |||
459 | 119 | |||
460 | 120 | def make_fake_upload(sourceful=False, binaryful=False): | ||
461 | 121 | return FakeNascentUpload(sourceful, binaryful) | ||
462 | 122 | |||
463 | 123 | |||
464 | 124 | def make_policy(accepted_type): | ||
465 | 125 | policy = AbstractUploadPolicy() | ||
466 | 126 | policy.accepted_type = accepted_type | ||
467 | 127 | return policy | ||
468 | 0 | 128 | ||
469 | === modified file 'lib/lp/archiveuploader/uploadpolicy.py' | |||
470 | --- lib/lp/archiveuploader/uploadpolicy.py 2010-08-25 17:14:46 +0000 | |||
471 | +++ lib/lp/archiveuploader/uploadpolicy.py 2010-08-25 17:14:48 +0000 | |||
472 | @@ -7,6 +7,7 @@ | |||
473 | 7 | 7 | ||
474 | 8 | __all__ = [ | 8 | __all__ = [ |
475 | 9 | "AbstractUploadPolicy", | 9 | "AbstractUploadPolicy", |
476 | 10 | "ArchiveUploadType", | ||
477 | 10 | "BuildDaemonUploadPolicy", | 11 | "BuildDaemonUploadPolicy", |
478 | 11 | "findPolicyByName", | 12 | "findPolicyByName", |
479 | 12 | "IArchiveUploadPolicy", | 13 | "IArchiveUploadPolicy", |
480 | @@ -14,6 +15,8 @@ | |||
481 | 14 | "UploadPolicyError", | 15 | "UploadPolicyError", |
482 | 15 | ] | 16 | ] |
483 | 16 | 17 | ||
484 | 18 | from textwrap import dedent | ||
485 | 19 | |||
486 | 17 | from zope.component import ( | 20 | from zope.component import ( |
487 | 18 | getGlobalSiteManager, | 21 | getGlobalSiteManager, |
488 | 19 | getUtility, | 22 | getUtility, |
489 | @@ -28,6 +31,9 @@ | |||
490 | 28 | from lp.registry.interfaces.pocket import PackagePublishingPocket | 31 | from lp.registry.interfaces.pocket import PackagePublishingPocket |
491 | 29 | from lp.registry.interfaces.series import SeriesStatus | 32 | from lp.registry.interfaces.series import SeriesStatus |
492 | 30 | 33 | ||
493 | 34 | from lazr.enum import EnumeratedType, Item | ||
494 | 35 | |||
495 | 36 | |||
496 | 31 | # Defined here so that uploadpolicy.py doesn't depend on lp.code. | 37 | # Defined here so that uploadpolicy.py doesn't depend on lp.code. |
497 | 32 | SOURCE_PACKAGE_RECIPE_UPLOAD_POLICY_NAME = 'recipe' | 38 | SOURCE_PACKAGE_RECIPE_UPLOAD_POLICY_NAME = 'recipe' |
498 | 33 | # Number of seconds in an hour (used later) | 39 | # Number of seconds in an hour (used later) |
499 | @@ -52,6 +58,13 @@ | |||
500 | 52 | """ | 58 | """ |
501 | 53 | 59 | ||
502 | 54 | 60 | ||
503 | 61 | class ArchiveUploadType(EnumeratedType): | ||
504 | 62 | |||
505 | 63 | SOURCE_ONLY = Item("Source only") | ||
506 | 64 | BINARY_ONLY = Item("Binary only") | ||
507 | 65 | MIXED_ONLY = Item("Mixed only") | ||
508 | 66 | |||
509 | 67 | |||
510 | 55 | class AbstractUploadPolicy: | 68 | class AbstractUploadPolicy: |
511 | 56 | """Encapsulate the policy of an upload to a launchpad archive. | 69 | """Encapsulate the policy of an upload to a launchpad archive. |
512 | 57 | 70 | ||
513 | @@ -63,8 +76,9 @@ | |||
514 | 63 | """ | 76 | """ |
515 | 64 | implements(IArchiveUploadPolicy) | 77 | implements(IArchiveUploadPolicy) |
516 | 65 | 78 | ||
517 | 79 | name = 'abstract' | ||
518 | 66 | options = None | 80 | options = None |
520 | 67 | name = 'abstract' | 81 | accepted_type = None # Must be defined in subclasses. |
521 | 68 | 82 | ||
522 | 69 | def __init__(self): | 83 | def __init__(self): |
523 | 70 | """Prepare a policy...""" | 84 | """Prepare a policy...""" |
524 | @@ -75,14 +89,45 @@ | |||
525 | 75 | self.unsigned_changes_ok = False | 89 | self.unsigned_changes_ok = False |
526 | 76 | self.unsigned_dsc_ok = False | 90 | self.unsigned_dsc_ok = False |
527 | 77 | self.create_people = True | 91 | self.create_people = True |
528 | 78 | self.can_upload_source = True | ||
529 | 79 | self.can_upload_binaries = True | ||
530 | 80 | self.can_upload_mixed = True | ||
531 | 81 | # future_time_grace is in seconds. 28800 is 8 hours | 92 | # future_time_grace is in seconds. 28800 is 8 hours |
532 | 82 | self.future_time_grace = 8 * HOURS | 93 | self.future_time_grace = 8 * HOURS |
533 | 83 | # The earliest year we accept in a deb's file's mtime | 94 | # The earliest year we accept in a deb's file's mtime |
534 | 84 | self.earliest_year = 1984 | 95 | self.earliest_year = 1984 |
535 | 85 | 96 | ||
536 | 97 | def validateUploadType(self, upload): | ||
537 | 98 | """Check that the type of the given upload is accepted by this policy. | ||
538 | 99 | |||
539 | 100 | When the type (e.g. sourceful, binaryful or mixed) is not accepted, | ||
540 | 101 | the upload is rejected. | ||
541 | 102 | """ | ||
542 | 103 | if upload.sourceful and upload.binaryful: | ||
543 | 104 | if self.accepted_type != ArchiveUploadType.MIXED_ONLY: | ||
544 | 105 | upload.reject( | ||
545 | 106 | "Source/binary (i.e. mixed) uploads are not allowed.") | ||
546 | 107 | |||
547 | 108 | elif upload.sourceful: | ||
548 | 109 | if self.accepted_type != ArchiveUploadType.SOURCE_ONLY: | ||
549 | 110 | upload.reject( | ||
550 | 111 | "Sourceful uploads are not accepted by this policy.") | ||
551 | 112 | |||
552 | 113 | elif upload.binaryful: | ||
553 | 114 | if self.accepted_type != ArchiveUploadType.BINARY_ONLY: | ||
554 | 115 | message = dedent(""" | ||
555 | 116 | Upload rejected because it contains binary packages. | ||
556 | 117 | Ensure you are using `debuild -S`, or an equivalent | ||
557 | 118 | command, to generate only the source package before | ||
558 | 119 | re-uploading.""") | ||
559 | 120 | |||
560 | 121 | if upload.is_ppa: | ||
561 | 122 | message += dedent(""" | ||
562 | 123 | See https://help.launchpad.net/Packaging/PPA for | ||
563 | 124 | more information.""") | ||
564 | 125 | upload.reject(message) | ||
565 | 126 | |||
566 | 127 | else: | ||
567 | 128 | raise AssertionError( | ||
568 | 129 | "Upload is not sourceful, binaryful or mixed.") | ||
569 | 130 | |||
570 | 86 | def getUploader(self, changes): | 131 | def getUploader(self, changes): |
571 | 87 | """Get the person who is doing the uploading.""" | 132 | """Get the person who is doing the uploading.""" |
572 | 88 | return changes.signer | 133 | return changes.signer |
573 | @@ -171,11 +216,7 @@ | |||
574 | 171 | """The insecure upload policy is used by the poppy interface.""" | 216 | """The insecure upload policy is used by the poppy interface.""" |
575 | 172 | 217 | ||
576 | 173 | name = 'insecure' | 218 | name = 'insecure' |
582 | 174 | 219 | accepted_type = ArchiveUploadType.SOURCE_ONLY | |
578 | 175 | def __init__(self): | ||
579 | 176 | AbstractUploadPolicy.__init__(self) | ||
580 | 177 | self.can_upload_binaries = False | ||
581 | 178 | self.can_upload_mixed = False | ||
583 | 179 | 220 | ||
584 | 180 | def rejectPPAUploads(self, upload): | 221 | def rejectPPAUploads(self, upload): |
585 | 181 | """Insecure policy allows PPA upload.""" | 222 | """Insecure policy allows PPA upload.""" |
586 | @@ -283,14 +324,13 @@ | |||
587 | 283 | """The build daemon upload policy is invoked by the slave scanner.""" | 324 | """The build daemon upload policy is invoked by the slave scanner.""" |
588 | 284 | 325 | ||
589 | 285 | name = 'buildd' | 326 | name = 'buildd' |
590 | 327 | accepted_type = ArchiveUploadType.BINARY_ONLY | ||
591 | 286 | 328 | ||
592 | 287 | def __init__(self): | 329 | def __init__(self): |
593 | 288 | super(BuildDaemonUploadPolicy, self).__init__() | 330 | super(BuildDaemonUploadPolicy, self).__init__() |
594 | 289 | # We permit unsigned uploads because we trust our build daemons | 331 | # We permit unsigned uploads because we trust our build daemons |
595 | 290 | self.unsigned_changes_ok = True | 332 | self.unsigned_changes_ok = True |
596 | 291 | self.unsigned_dsc_ok = True | 333 | self.unsigned_dsc_ok = True |
597 | 292 | self.can_upload_source = False | ||
598 | 293 | self.can_upload_mixed = False | ||
599 | 294 | 334 | ||
600 | 295 | def setOptions(self, options): | 335 | def setOptions(self, options): |
601 | 296 | AbstractUploadPolicy.setOptions(self, options) | 336 | AbstractUploadPolicy.setOptions(self, options) |
602 | @@ -314,15 +354,13 @@ | |||
603 | 314 | """This policy is invoked when processing sync uploads.""" | 354 | """This policy is invoked when processing sync uploads.""" |
604 | 315 | 355 | ||
605 | 316 | name = 'sync' | 356 | name = 'sync' |
606 | 357 | accepted_type = ArchiveUploadType.SOURCE_ONLY | ||
607 | 317 | 358 | ||
608 | 318 | def __init__(self): | 359 | def __init__(self): |
609 | 319 | AbstractUploadPolicy.__init__(self) | 360 | AbstractUploadPolicy.__init__(self) |
610 | 320 | # We don't require changes or dsc to be signed for syncs | 361 | # We don't require changes or dsc to be signed for syncs |
611 | 321 | self.unsigned_changes_ok = True | 362 | self.unsigned_changes_ok = True |
612 | 322 | self.unsigned_dsc_ok = True | 363 | self.unsigned_dsc_ok = True |
613 | 323 | # We don't want binaries in a sync | ||
614 | 324 | self.can_upload_mixed = False | ||
615 | 325 | self.can_upload_binaries = False | ||
616 | 326 | 364 | ||
617 | 327 | def policySpecificChecks(self, upload): | 365 | def policySpecificChecks(self, upload): |
618 | 328 | """Perform sync specific checks.""" | 366 | """Perform sync specific checks.""" |
619 | 329 | 367 | ||
620 | === modified file 'lib/lp/code/model/sourcepackagerecipebuild.py' | |||
621 | --- lib/lp/code/model/sourcepackagerecipebuild.py 2010-08-24 09:51:26 +0000 | |||
622 | +++ lib/lp/code/model/sourcepackagerecipebuild.py 2010-08-25 17:14:48 +0000 | |||
623 | @@ -40,6 +40,7 @@ | |||
624 | 40 | from canonical.launchpad.webapp import errorlog | 40 | from canonical.launchpad.webapp import errorlog |
625 | 41 | from lp.app.errors import NotFoundError | 41 | from lp.app.errors import NotFoundError |
626 | 42 | from lp.archiveuploader.uploadpolicy import ( | 42 | from lp.archiveuploader.uploadpolicy import ( |
627 | 43 | ArchiveUploadType, | ||
628 | 43 | BuildDaemonUploadPolicy, | 44 | BuildDaemonUploadPolicy, |
629 | 44 | IArchiveUploadPolicy, | 45 | IArchiveUploadPolicy, |
630 | 45 | SOURCE_PACKAGE_RECIPE_UPLOAD_POLICY_NAME, | 46 | SOURCE_PACKAGE_RECIPE_UPLOAD_POLICY_NAME, |
631 | @@ -78,11 +79,7 @@ | |||
632 | 78 | """Policy for uploading the results of a source package recipe build.""" | 79 | """Policy for uploading the results of a source package recipe build.""" |
633 | 79 | 80 | ||
634 | 80 | name = SOURCE_PACKAGE_RECIPE_UPLOAD_POLICY_NAME | 81 | name = SOURCE_PACKAGE_RECIPE_UPLOAD_POLICY_NAME |
640 | 81 | 82 | accepted_type = ArchiveUploadType.SOURCE_ONLY | |
636 | 82 | def __init__(self): | ||
637 | 83 | super(SourcePackageRecipeUploadPolicy, self).__init__() | ||
638 | 84 | self.can_upload_source = True | ||
639 | 85 | self.can_upload_binaries = False | ||
641 | 86 | 83 | ||
642 | 87 | def getUploader(self, changes): | 84 | def getUploader(self, changes): |
643 | 88 | """Return the person doing the upload.""" | 85 | """Return the person doing the upload.""" |
644 | 89 | 86 | ||
645 | === modified file 'lib/lp/soyuz/browser/tests/distroseriesqueue-views.txt' | |||
646 | --- lib/lp/soyuz/browser/tests/distroseriesqueue-views.txt 2010-08-20 12:11:30 +0000 | |||
647 | +++ lib/lp/soyuz/browser/tests/distroseriesqueue-views.txt 2010-08-25 17:14:48 +0000 | |||
648 | @@ -253,6 +253,7 @@ | |||
649 | 253 | ... sourcename="foo", distroseries=hoary, version="1.0-2", | 253 | ... sourcename="foo", distroseries=hoary, version="1.0-2", |
650 | 254 | ... status=PackagePublishingStatus.PUBLISHED) | 254 | ... status=PackagePublishingStatus.PUBLISHED) |
651 | 255 | 255 | ||
652 | 256 | >>> from lp.archiveuploader.uploadpolicy import ArchiveUploadType | ||
653 | 256 | >>> from canonical.launchpad.ftests import import_public_test_keys | 257 | >>> from canonical.launchpad.ftests import import_public_test_keys |
654 | 257 | >>> from canonical.launchpad.interfaces import IComponentSet | 258 | >>> from canonical.launchpad.interfaces import IComponentSet |
655 | 258 | >>> from lp.soyuz.model.component import ComponentSelection | 259 | >>> from lp.soyuz.model.component import ComponentSelection |
656 | @@ -264,7 +265,7 @@ | |||
657 | 264 | >>> trash = ComponentSelection(distroseries=hoary, component=universe) | 265 | >>> trash = ComponentSelection(distroseries=hoary, component=universe) |
658 | 265 | >>> sync_policy = getPolicy(name='sync', distro='ubuntu', | 266 | >>> sync_policy = getPolicy(name='sync', distro='ubuntu', |
659 | 266 | ... distroseries='hoary') | 267 | ... distroseries='hoary') |
661 | 267 | >>> sync_policy.can_upload_binaries = True | 268 | >>> sync_policy.accepted_type = ArchiveUploadType.BINARY_ONLY |
662 | 268 | >>> foo_upload = NascentUpload.from_changesfile_path( | 269 | >>> foo_upload = NascentUpload.from_changesfile_path( |
663 | 269 | ... datadir('suite/foo_1.0-2_multi_binary/foo_1.0-2_i386.changes'), | 270 | ... datadir('suite/foo_1.0-2_multi_binary/foo_1.0-2_i386.changes'), |
664 | 270 | ... sync_policy, mock_logger_quiet) | 271 | ... sync_policy, mock_logger_quiet) |
665 | 271 | 272 | ||
666 | === modified file 'lib/lp/soyuz/doc/soyuz-set-of-uploads.txt' | |||
667 | --- lib/lp/soyuz/doc/soyuz-set-of-uploads.txt 2010-08-06 14:29:36 +0000 | |||
668 | +++ lib/lp/soyuz/doc/soyuz-set-of-uploads.txt 2010-08-25 17:14:48 +0000 | |||
669 | @@ -319,22 +319,6 @@ | |||
670 | 319 | Subject: bar_1.0-3_source.changes rejected | 319 | Subject: bar_1.0-3_source.changes rejected |
671 | 320 | ... | 320 | ... |
672 | 321 | 321 | ||
673 | 322 | Check the rejection (instead of failure) of an upload to a series or | ||
674 | 323 | pocket which does not exist. We use the security upload policy for easier | ||
675 | 324 | testing, as unsigned uploads are allowed. | ||
676 | 325 | |||
677 | 326 | >>> simulate_upload( | ||
678 | 327 | ... 'unstable_1.0-1', upload_policy="security", loglevel=logging.ERROR) | ||
679 | 328 | Rejected uploads: ['unstable_1.0-1'] | ||
680 | 329 | |||
681 | 330 | >>> read_email() | ||
682 | 331 | To: Celso Providelo <celso.providelo@canonical.com> | ||
683 | 332 | Subject: unstable_1.0-1_source.changes rejected | ||
684 | 333 | ... | ||
685 | 334 | Rejected: | ||
686 | 335 | Unable to find distroseries: unstable | ||
687 | 336 | ... | ||
688 | 337 | |||
689 | 338 | Force weird behavior with rfc2047 sentences containing '.' on | 322 | Force weird behavior with rfc2047 sentences containing '.' on |
690 | 339 | bar_1.0-4, which caused bug # 41102. | 323 | bar_1.0-4, which caused bug # 41102. |
691 | 340 | 324 | ||
692 | @@ -605,6 +589,9 @@ | |||
693 | 605 | 589 | ||
694 | 606 | == Staged Security Uploads == | 590 | == Staged Security Uploads == |
695 | 607 | 591 | ||
696 | 592 | XXX: Salgado, 2010-08-25, bug=624078: This entire section should be removed | ||
697 | 593 | but if you do so publish-distro.py (called further down) will hang. | ||
698 | 594 | |||
699 | 608 | Perform a staged (source-first) security upload, simulating a run of | 595 | Perform a staged (source-first) security upload, simulating a run of |
700 | 609 | the buildd-queuebuilder inbetween, to verify fix for bug 53437. To be | 596 | the buildd-queuebuilder inbetween, to verify fix for bug 53437. To be |
701 | 610 | allowed a security upload, we need to use a released distroseries, | 597 | allowed a security upload, we need to use a released distroseries, |
702 | @@ -617,7 +604,7 @@ | |||
703 | 617 | >>> ss = SectionSelection(distroseries=warty, section=devel) | 604 | >>> ss = SectionSelection(distroseries=warty, section=devel) |
704 | 618 | 605 | ||
705 | 619 | >>> simulate_upload( | 606 | >>> simulate_upload( |
707 | 620 | ... 'baz_1.0-1', is_new=True, upload_policy="security", | 607 | ... 'baz_1.0-1', is_new=True, upload_policy="anything", |
708 | 621 | ... series="warty-security", distro="ubuntu") | 608 | ... series="warty-security", distro="ubuntu") |
709 | 622 | 609 | ||
710 | 623 | Check there's a SourcePackageRelease with no build. | 610 | Check there's a SourcePackageRelease with no build. |
711 | @@ -650,7 +637,7 @@ | |||
712 | 650 | Upload the i386 binary: | 637 | Upload the i386 binary: |
713 | 651 | 638 | ||
714 | 652 | >>> simulate_upload( | 639 | >>> simulate_upload( |
716 | 653 | ... 'baz_1.0-1_single_binary', is_new=True, upload_policy="security", | 640 | ... 'baz_1.0-1_single_binary', is_new=True, upload_policy="anything", |
717 | 654 | ... distro="ubuntu", series="warty-security") | 641 | ... distro="ubuntu", series="warty-security") |
718 | 655 | 642 | ||
719 | 656 | Should still just have one build, and it should now be FULLYBUILT. | 643 | Should still just have one build, and it should now be FULLYBUILT. |
Beautiful. A few notes, of course:
Lines 199 and 319 of the diff have multiple imports that don't conform to the new import formatting rules:
from foobar import (
Bar,
Foo,
)
In line 379 of the diff, the "else" and the assertion throw the symmetry of the code slightly out of whack. Just an idea, but maybe you can move it out into a separate "else" clause?
elif upload.binaryful: "...")
# [...]
else:
raise AssertionError(
The list of hand-wrapped text lines at line 382 of the diff puzzles me. Why not a single dedent( """triple- quoted multi-line string""")?
Finally, I think I see either an indentation problem or something involving tabs around line 390. Did you run "make lint" over the current form of this branch?
But that's all small fry. The branch is definitely an improvement.