Merge lp://staging/~matsubara/tarmac/bundle-lander into lp://staging/~launchpad/tarmac/lp-tarmac
Proposed by
Diogo Matsubara
Status: | Merged |
---|---|
Approved by: | Diogo Matsubara |
Approved revision: | 398 |
Merged at revision: | 384 |
Proposed branch: | lp://staging/~matsubara/tarmac/bundle-lander |
Merge into: | lp://staging/~launchpad/tarmac/lp-tarmac |
Diff against target: |
834 lines (+661/-35) 7 files modified
tarmac/bin/commands.py (+20/-0) tarmac/pidfile.py (+77/-0) tarmac/plugins/bundlecommand.py (+215/-30) tarmac/plugins/tests/test_bundlecommand.py (+280/-0) tarmac/tests/mock.py (+17/-0) tarmac/tests/test_commands.py (+21/-5) tarmac/tests/test_pidfile.py (+31/-0) |
To merge this branch: | bzr merge lp://staging/~matsubara/tarmac/bundle-lander |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Gary Poster (community) | Approve | ||
Review via email:
|
Commit message
Implements a lock mechanism and a way to properly format the test results similar to the way they are formatted in ec2test and email those results to merge proposal subscribers.
Description of the change
This branch implements a simple lock mechanism for the bundle merge command and implements a way to properly format the test resuls similar to the way they are formatted in ec2test and email those results to merge proposal subscribers.
All tests pass.
To post a comment you must log in.
Lock file: here are some things that would be nice, in decreasing order of importance in my mind.
1 lock file is actually a pid file. pid is reported in error message. This gives SAs a chance to quickly see if it is dead or not.
2 if lock file exists and has been around for fewer than N seconds, exit with status code 0. If lock file has been around for greater than or equal to N seconds, exit with a non-0 status code. N should be configurable. This gives SAs a chance to care about serious problems, and ignore typical problems.
3 if lock file exists, error includes how long it has been around. That's easy enough to do without (just look at the file yourself), but a nice convenience.
4 if lock file exists, with pid, and identified pid doesn't exist, delete the pid file, mention what you are doing, and proceed to run tarmac.
FWIW, lib/canonical/ lazr/pidfile. py has some simple code to steal for #1.
I think I'll say that I want #1 unless you convince me otherwise. I'd be happy if you threw in #2 and #3, but I'm fine with ignoring it for now. I don't think you should pursue #4 right now.
Email: cool. A small note is that "(See the attached file for the complete log)" will be included in the merge proposal output, and that will be incorrect. Might be mildly confusing. Would be nice to omit it in the merge proposal, and only include it when sending the mail.
Nice tests. It test_run_failure, is it possible/reasonable to show that the merge proposal was changed, and that emails were sent? That seems like it would be nice.