Comment 1 for bug 1538573

Hi,

We would like to thank you for your time and effort you put into this charm.

It would be great if you could submit the repo with the layer you implemented in addition to the charm build output? This would make the review process much easier.
Guess it is this one https://code.launchpad.net/~jacekn/canonical-is-charms/collectd-composer , right?

In respect to your tests failing, we found the problem to be in line: http://bazaar.launchpad.net/~jacekn/charms/trusty/collectd/trunk/view/head:/tests/99-autogen#L14
You should a) be referencing explicitely the relation name for ubuntu and b) use the interface name of collectd.
So the line at hand should look like this: cls.deployment.relate('ubuntu:juju-info', 'collectd:host')

Can you also please make sure your "make test" is successful? You could use bundletester https://github.com/juju-solutions/bundletester to ensure your charm is ready.

Here are some additional comments we find important:

- We do not recomend using a hook to trap config changes.
  Many thanks to Cory Johns who tried to refactor your charm:
  https://code.launchpad.net/~johnsca/charms/trusty/collectd/layer-rq

- For subordinate charms we recommend you use the option:
  options:
    basic:
      use_venv: true
  In this way your subordinate charm will not conflict with the pricipal one as it will operate in a virtual environment.
  Have a look here: https://github.com/johnsca/layer-apache-hadoop-plugin/commit/861031a3a07a03ed6589065d1c59c40cbd1cdf9e

Many thanks to Cary John, and Kevin Monroe who helped in the review.

Thank you for your time,
Konstantinos