Juju Charms Collection
rabbitmq-server package

Code review comment for lp:~james-page/charms/trusty/rabbitmq-server/status-check

  1. Trusty Tahr (14.04)
  2. status-check
  3. Merge into next
Revision history for this message
David Ames (thedac) wrote :

I really like the workgroup status approach in this MP.

I also like your solution to https://bugs.launchpad.net/charms/+source/rabbitmq-server/+bug/1501048 better than mine.
mkdir is imported in hooks/rabbitmq_server_relations.py but is never used. I suspect you used os.path.exists instead.

I also really like breaking out clustered() as a cached function on its own. However, one of the bugs I am fighting (https://bugs.launchpad.net/charms/+source/rabbitmq-server/+bug/1500204) is a direct result of checking for more than one running node.

if len(running_nodes()) > 1:

Consider, the 3rd, 4th and 5th nodes to attempt clustering. There will already by 2+ running nodes and they will assume incorrectly they are already clustered.

In my testing I have removed this check entirely with some success. I would be interested in finding a more robust clustered check.

I plan to merge your MP into my branch and do some more testing and see if I can come up with a clustered check that works in all cases.

review: Needs Fixing

« Back to merge proposal