Merge lp://staging/~psivaa/charms/trusty/jenkaas/example-job into lp://staging/~canonical-ci-engineering/charms/trusty/jenkaas/trunk

Proposed by Para Siva
Status: Merged
Approved by: Para Siva
Approved revision: 17
Merged at revision: 7
Proposed branch: lp://staging/~psivaa/charms/trusty/jenkaas/example-job
Merge into: lp://staging/~canonical-ci-engineering/charms/trusty/jenkaas/trunk
Diff against target: 162 lines (+84/-29)
4 files modified
files/templates/example_job/config.xml (+40/-0)
hooks/actions.py (+37/-1)
hooks/services.py (+7/-8)
templates/upstart.conf (+0/-20)
To merge this branch: bzr merge lp://staging/~psivaa/charms/trusty/jenkaas/example-job
Reviewer Review Type Date Requested Status
Francis Ginther Approve
Evan (community) Needs Information
Joe Talbott (community) Approve
Review via email: mp+262129@code.staging.launchpad.net

Commit message

Create an example job from a simple template. Using start and stop helpers.

Description of the change

Create an example job from a simple template. This can now be deployed. There are races as to when it's actually possible to create the example job, pragmatically.

I'm not sure if risking this race for an example job creation, which does not imho add a lot of values.

http://162.213.33.201:8080/job/example_job/

is the job that was created using this MP

To post a comment you must log in.
12. By Para Siva

Use service stop start for example job creation

13. By Para Siva

Rename template

Revision history for this message
Joe Talbott (joetalbott) wrote :

This looks good to me.

review: Approve
Revision history for this message
Francis Ginther (fginther) wrote :

A couple of comments on the jenkins service stop and start when updating the job templates. I think you've already discussed doing this differently in IRC.

review: Needs Fixing
14. By Para Siva

Block jenkins service starting whilst deb installations via policy-rc.d

Revision history for this message
Para Siva (psivaa) wrote :

Thanks for the reviews. Rev 14 is now using policy-rc.d to stop jenkins from starting. Could you please re-review?

Revision history for this message
Evan (ev) :
review: Needs Fixing
Revision history for this message
Francis Ginther (fginther) wrote :

Looks good. I would change the missing "#!/bin/sh" though. I agree it isn't strictly necessary, but it will more then likely lead to confusion if it's not there.

review: Approve
15. By Para Siva

Add shebang to the polic-rc.d

Revision history for this message
Para Siva (psivaa) wrote :

"#!/bin/sh" is now added to the policy-rc.d. Thanks for the reviews.

Revision history for this message
Francis Ginther (fginther) wrote :

Approve

review: Approve
Revision history for this message
Joe Talbott (joetalbott) wrote :

LGTM

review: Approve
Revision history for this message
Evan (ev) wrote :

Mostly looks good, but I don't understand the point of the upstart job.

review: Needs Information
16. By Para Siva

Remove upstart job and use service_start to start jenkins

17. By Para Siva

Remove empty folder

Revision history for this message
Para Siva (psivaa) wrote :

Thanks. The upstart job was to follow the 'pattern' of the other charms that we were using. Now removed it and using service start.

Revision history for this message
Francis Ginther (fginther) wrote :

Siva, I'm all for merging this now. The removal of the upstart job and service_start() look good.

review: Approve
Revision history for this message
Para Siva (psivaa) wrote :

Thanks Francis. I'll go ahead and merge it. If Evan has further comments, i'll address them in another MP.

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