VIVO charm

Some feedback:

The charm depends on wget which is not installed in lxc containers . To make sure that the charm works regardless of where it is installed, it would be a good idea to explicitly install wget.

-Juan

Hi Nicholas, thanks for submitting a charm!

VIVO looks really cool, I deployed and it seemed to work, though it complained about not having SOLR (dunno if you know, but SOLR *was* just promulgated into the store. :)

== Blocking Changes ==

* You need to cryptographically verify the download from sourceforge. Currently you are trusting that sourceforge.net has not been compromised by a man in the middle.

Since you are just linking directly to v1.4.1, I suggest just putting the tarball in the charm itself. This means you will need to update the charm whenever there are new upstream releases, but you are already in that boat.

* install is not idempotent - install is only run once per deploy, but *sometimes* install has errors, and will have to be retried with 'juju resolved --retry vivo-service-name/0'. Any steps that are not repeatable will fail and cause problems. You use 'mkdir' without '-p' quite a few times, and these will exit with error codes in a second run.

A good way to test for idempotency is to deploy, immediately followd by 'juju debug-hooks vivo/0', and then you can just run the hook twice when the 'install' window opens in tmux/byobu. A good guideline to use is mkdir always gets '-p', mv, rm and cp always get '-f'.

* relation-get in config-changed will *always* fail. relation-get without any qualification is only valid in the *-relation-* hooks, unlesss you pass '-r' to set the relation-id. Instead you'll need to do something like this:

dbrel_id=`relation-ids|head -1`
if [ -z "$dbrel_id" ] ; then
  exit 0
fi
database=`relation-get -r $dbrel_id database`

You probably should also make it clear that you can only relate to one database service at a time, given that if you related to two, which one is chosen (above) is somewhat undefined.

Can I get another review? How do I request one? :-)

nicholas, if you fixed the commented issues, you can change the bug status to "fix committed" or "new" and it'll be back in the queue

Nathan thank you. Somehow I missed your comment, so this sat for a bit. Ohh well, it's still ready to be looked at :-)

Hi Nicholas!

The changes look great. In the mean time since you did your changes, we added a new policy item. Charms must have a maintainer.

$ charm proof
E: Charms need a maintainer (See RFC2822) - Name <email>
W: missing recommended hook start
W: missing recommended hook stop

Also those warnings should be fixed. While start/stop don't currently get called all that often, in the near future charms will use these for things like live migration and backups. Please make sure you have a working start/stop hook.

Fix those three things from charm proof and this one is definitely ready for the store!

When you've fixed them, please re-subscribe 'charmers' to this bug and somebody will promulgate ASAP.