Merge lp:~wallyworld/launchpad/recipe-build-removed-recipe into lp:launchpad
Status: | Merged | ||||
---|---|---|---|---|---|
Approved by: | Leonard Richardson | ||||
Approved revision: | no longer in the source branch. | ||||
Merged at revision: | 12304 | ||||
Proposed branch: | lp:~wallyworld/launchpad/recipe-build-removed-recipe | ||||
Merge into: | lp:launchpad | ||||
Diff against target: |
151 lines (+80/-7) 4 files modified
lib/lp/archiveuploader/tests/test_uploadprocessor.py (+34/-0) lib/lp/archiveuploader/uploadprocessor.py (+26/-7) lib/lp/code/model/sourcepackagerecipebuild.py (+4/-0) lib/lp/code/model/tests/test_sourcepackagerecipebuild.py (+16/-0) |
||||
To merge this branch: | bzr merge lp:~wallyworld/launchpad/recipe-build-removed-recipe | ||||
Related bugs: |
|
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Leonard Richardson (community) | Approve | ||
Review via email: mp+48103@code.launchpad.net |
Commit message
[r=leonardr]
Description of the change
See bug 698032. When a recipe build has been removed since a recipe build was started the archive uploader will fall over.
== Implementation ==
Add a check to the upload processor to see if a recipe has been deleted. Not quite as simple as that, since there's a potential race condition whereby any check placed at the beginning of the process could pass and the recipe could then be deleted during the processing (the effects will depend on the transaction boundaries etc). So, the implementation is to check at the beginning and again at the end, deferring any error processing till the last check. The error processing is to log a warning. Any post build clean up is done as per a build failure to ensure any files are not left lying around.
A check was also added to the mail notification method of SourcePackageRe
== Tests ==
Addded a new test to lp.archiveloade
- testSourcePacka
== Lint ==
Checking for conflicts and issues in changed files.
Linting changed files:
lib/lp/
lib/lp/
lib/lp/
./lib/lp/
1263: E301 expected 1 blank line, found 2
To clarify the implementation:
We want to stop the oops when upload processing is attempted for a build with a deleted recipe. We don't want to email any error message but rather we just want to log a warning. We also need to ensure any normal post upload cleanup is done (as per for the standard upload failure case).
So the approach was to check at the start of the process() method to see if the recipe was deleted and flag it, don't attempt the upload processing, and defer the handling of the problem to occur along with other failure mode processing (so that all error handling is done in the one place).
The comments about the transaction boundaries are probably not relevant - it appears the upload processing is wrapped inside a transaction so there's no likelihood of any race condition. But the implementation would handle this case too.