Comment 2 for bug 996576

Clint Byrum (clint-fewbar) wrote :

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 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
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.