Merge lp://staging/~doanac/ubuntu-ci-services-itself/run-worker-conversion into lp://staging/ubuntu-ci-services-itself

Proposed by Andy Doan
Status: Merged
Merged at revision: 368
Proposed branch: lp://staging/~doanac/ubuntu-ci-services-itself/run-worker-conversion
Merge into: lp://staging/ubuntu-ci-services-itself
Prerequisite: lp://staging/~doanac/ubuntu-ci-services-itself/runworker-cancellable
Diff against target: 453 lines (+112/-220)
4 files modified
branch-source-builder/bsbuilder/run_worker.py (+30/-51)
ci-utils/ci_utils/amqp_worker.py (+7/-4)
image-builder/imagebuilder/run_worker.py (+15/-84)
test_runner/tstrun/run_worker.py (+60/-81)
To merge this branch: bzr merge lp://staging/~doanac/ubuntu-ci-services-itself/run-worker-conversion
Reviewer Review Type Date Requested Status
Vincent Ladeuil (community) Approve
PS Jenkins bot (community) continuous-integration Needs Fixing
Review via email: mp+210078@code.staging.launchpad.net

Commit message

convert the run-workers over to the new common run-worker helper

Description of the change

Part 3 of the 3 part series to improve our run-workers and allow them to be cancelled.

This is mostly identation changes and deleted lines, but the diff view is the most friendly. It might be easier to just read each run_worker.py file as a whole.

NOTE: I've removed a long fix-me in the test-runner:
286 - # FIXME: There is a potentially confusing failure mode here: by reusing
287 - # the same testbed for all packages, we may miss bad dependencies for
288 - # one package because that missing dependency is provided by another
289 - # package. It's vague and unclear that we should care about this edge
290 - # case at this point -- vila 2014-02-01

I think this needs to be opened as a bug

To post a comment you must log in.
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

PASSED: Continuous integration, rev:346
http://s-jenkins.ubuntu-ci:8080/job/uci-engine-ci/349/
Executed test runs:

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/uci-engine-ci/349/rebuild

review: Approve (continuous-integration)
Revision history for this message
Vincent Ladeuil (vila) wrote :

> NOTE: I've removed a long fix-me in the test-runner:
> I think this needs to be opened as a bug

bug # ? :-p Or was I over concerned ?

64 + if params.get('cancelled'):

Oh right advisory is needed here, glad I approved the previous one ;)

84 - # remove from queue so request becomes completed

Hey, that's one of the comments I wanted in part 1/3 !

=== modified file 'image-builder/imagebuilder/run_worker.py'

/me blinks, hold on, where is the logger used here ?

313 - # FIXME: There is still a hole somewhere, the ack below won't happen if
314 - # an exception occurs while calling 'notify' just above, tricky testing
315 - # ahead... -- vila 2014-02-06

That one is dead for good ! Well done and thanks ;)

+1

review: Approve
Revision history for this message
Andy Doan (doanac) wrote :

Lets hold off landing this until both of these have gone to trunk:

1) https://code.launchpad.net/~doanac/ubuntu-ci-services-itself/bsbuilder-better-error-handling/+merge/210312
2) https://code.launchpad.net/~doanac/ubuntu-ci-services-itself/cancel-url/+merge/210075

specifically 1 will cause conflicts, but should be easy to clean up and re-test

347. By Andy Doan

merged with changes from trunk

this required fixing conflicts now that the workers were changed
to include artifacts

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

PASSED: Continuous integration, rev:347
http://s-jenkins.ubuntu-ci:8080/job/uci-engine-ci/383/
Executed test runs:

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/uci-engine-ci/383/rebuild

review: Approve (continuous-integration)
Revision history for this message
Andy Doan (doanac) wrote :

vila - this has been re-merged with trunk. i've been testing at home all night. Care to take another look

348. By Andy Doan

get logging output names consistent again

Get this back to the pattern fginther introduced while i was doing
this change

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

FAILED: Continuous integration, rev:348
http://s-jenkins.ubuntu-ci:8080/job/uci-engine-ci/389/
Executed test runs:

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/uci-engine-ci/389/rebuild

review: Needs Fixing (continuous-integration)
Revision history for this message
Vincent Ladeuil (vila) wrote :

355 + url = store.put_file(name, stream, 'text/plain')

Yeehaaa !

<vila> doanac: 7 failures in http://s-jenkins.ubuntu-ci:8080/job/uci-engine-ci/389/console for https://code.launchpad.net/~doanac/ubuntu-ci-services-itself/run-worker-conversion/+merge/210078
<vila> doanac: the fix is probably trivial, super(_worker, self).__init__()
<vila> TypeError: __init__() takes exactly 2 arguments (1 given)
<vila> doanac: additionally, since you played with it, it means it's a test-only issue. If you're peacefully sleeping, I'll try to land it with ev or psivaa help

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.

Subscribers

People subscribed via source and target branches