Merge lp:~jml/libdep-service/juju into lp:libdep-service
| Status: | Merged |
|---|---|
| Merged at revision: | 12 |
| Proposed branch: | lp:~jml/libdep-service/juju |
| Merge into: | lp:libdep-service |
| Diff against target: |
773 lines (+564/-80) 23 files modified
.bzrignore (+1/-0) charms/precise/libdep-service/README (+38/-0) charms/precise/libdep-service/copyright (+17/-0) charms/precise/libdep-service/dev-config/apply (+52/-0) charms/precise/libdep-service/dev-config/manifests/libdep_service.pp (+110/-0) charms/precise/libdep-service/dev-config/templates/django.wsgi.erb (+21/-0) charms/precise/libdep-service/dev-config/templates/httpd.conf.erb (+12/-0) charms/precise/libdep-service/dev-config/templates/nondev_credentials.cfg.erb (+19/-0) charms/precise/libdep-service/dev-config/templates/pkgme-devportal.conf.erb (+3/-0) charms/precise/libdep-service/hooks/install (+92/-0) charms/precise/libdep-service/hooks/relation-name-relation-broken (+2/-0) charms/precise/libdep-service/hooks/relation-name-relation-changed (+9/-0) charms/precise/libdep-service/hooks/relation-name-relation-departed (+5/-0) charms/precise/libdep-service/hooks/relation-name-relation-joined (+5/-0) charms/precise/libdep-service/hooks/start (+4/-0) charms/precise/libdep-service/hooks/stop (+7/-0) charms/precise/libdep-service/metadata.yaml (+6/-0) charms/precise/libdep-service/revision (+1/-0) django_project/dev.cfg (+18/-0) django_project/main.cfg (+70/-0) django_project/manage.py (+9/-0) django_project/schema.py (+13/-0) django_project/settings.py (+50/-80) |
| To merge this branch: | bzr merge lp:~jml/libdep-service/juju |
| Related bugs: |
| Reviewer | Review Type | Date Requested | Status |
|---|---|---|---|
| James Westby (community) | 2012-06-21 | Approve on 2012-06-22 | |
| Clint Byrum (community) | 2012-06-21 | Needs Fixing on 2012-06-21 | |
|
Review via email:
|
|||
Commit Message
Add a Juju charm for deploying libdep-service.
Description of the Change
Juju charm for deploying libdep-service.
It's a bit rough-and-ready at this stage, but it works, and that's a good point for review.
Derived from lp:pkgme-service's fabric deploy task. I stripped out everything that pkgme-service has that libdep-service doesn't (oops, preflight, postgres support). All of these things will need to be added back in eventually.
Obvious things I'm missing right now:
* making poor use of juju's config, partly because I don't know how, more because I don't know why
* really bad use of interfaces
The branch also converts libdep-service to use django-configglue. Arguably this should be put into a separate branch. I'd be happy to do that if anyone else actually cares.
The XXXs should all be self-explanatory. Feedback on any of them would be appreciated.
Obviously this duplicates stuff (e.g. add_if_present) from lp:pkgme-service, which is bad. Not sure what to do about that.
I'm quite new to puppet, wsgi, django and juju, so I'm guessing there's plenty to improve.
http://
Thanks,
jml
| James Westby (james-w) wrote : | # |
170 +file { "$::basedir/
Is that actually written to by pkgme-devportal (or any other code)? Not so
important, but I don't think we currently log anything in that code.
311 +# XXX: Would be great to depend on more stable URLs. e.g. /+branch/foo.
You can just use
bzr branch https:/
392 +# XXX: Is there a better way to do this ``cd`` business?
393 +BASE_DIR=${PWD}
394 +cd libdep-
Doing this as
(cd libdep-
does the cd in a sub-shell, and so the main shell's CWD is unaffected,
saving the cd back.
750 + # TODO: remove the production_
751 + # longer in use on production. It's left here now to ease rollouts.
752 + if not add_if_
That's gone in pkgme-service trunk, and isn't needed here.
This is shaping up pretty well to get something green.
Thanks,
James
- 38. By Jonathan Lange on 2012-06-22
-
Remove interfaces.
- 39. By Jonathan Lange on 2012-06-22
-
Add a copyright file to cure lint warnings. Don't worry, we'll GPL soon.
- 40. By Jonathan Lange on 2012-06-22
-
Better way of getting the public host name.
- 41. By Jonathan Lange on 2012-06-22
-
Strip all the sudo, hopefully revealing Juju's own native DEBIAN_
FRONTEND= noninteractive
Run 'cd' commands in a subshell - 42. By Jonathan Lange on 2012-06-22
-
Right revision number. Actually drop all the 'sudo'.
- 43. By Jonathan Lange on 2012-06-22
-
More stable URLs for branches.
- 44. By Jonathan Lange on 2012-06-22
-
Remove not-needed production_
credentials fallback. - 45. By Jonathan Lange on 2012-06-22
-
Last cleanup
- 46. By Jonathan Lange on 2012-06-22
-
Rebase revision
| Jonathan Lange (jml) wrote : | # |
Thanks Clint & James. I've applied the suggested fixes. Would appreciate follow-up.
Note that the goal is not to enter the charm store yet. We need a useful service for that first.
- 47. By Jonathan Lange on 2012-06-22
-
Correct license file for Canonical charms.
| James Westby (james-w) wrote : | # |
Hi,
I think this looks good as a first cut now, thanks.
a "bzr ignore sourcecode/*" would be appreciated by webops.
Thanks,
James
- 48. By Jonathan Lange on 2012-06-22
-
The webops will appreciate this, I'm told.
- 49. By Jonathan Lange on 2012-06-22
-
Correct copyright.
| Jonathan Lange (jml) wrote : | # |
Done.

First, you will want to run 'charm proof' to see any policy or format errors:
E: no copyright file
E: template relations should be renamed to fit charm: relation-name
E: template interface names should be changed: interface-name
E: template relations should be renamed to fit charm: relation-name
E: template interface names should be changed: interface-name
E: template relations should be renamed to fit charm: relation-name
E: template interface names should be changed: interface-name
All of those need to be addressed before charm store inclusion.
For the interfaces, better to leave those out entirely until you know what they are and can provide an implementation.
* hooks/install: DEBIAN_ FRONTEND= noninteractive is assumed to be set now in all charms (since the version in Ubuntu precise anyway).
* hooks/install: better way to do cd's is to use pushd/popd so you don'at have to track BASE_DIR. Otherwise, no.
* dev-config/apply: get_public_ hostname( ) can be deleted, as PUBLIC_HOSTNAME must be changed to just $(unit-get public-address). This way it will work the same on all providers.
* dev-config/apply: I love the way puppet is used, as this is far more reliable than hand-coded shell to do everything in an idempotent way. I do think the whole package list from install could also be put into the puppet manifest.
* hooks/start, stop: These are actually important for the future. Eventually they'll be used when units are migrated or rebooted. Make sure to consider what needs to be run in those instances and put them here.
Otherwise, this looks great. You might consider just embedding exports in the charm, rather than bzr branching from launchpad, as that will eliminate a dependency to have launchpad available.