Code review comment for lp:~verterok/charms/trusty/haproxy/restart-rsyslog-after-install

Revision history for this message
Kevin W Monroe (kwmonroe) wrote :

Hi Guillermo,

In juju-1.25, the first unit number for a service may not always be 0. If you deploy haproxy for the first time, you'll get haproxy/0. If you then destroy haproxy and re-deploy it, you'll get haproxy/1. Because of this, I tweaked your 10_deploy_test.py like this:

-haproxy_unit = d.sentry.unit['haproxy/0']
+haproxy_unit = d.sentry['haproxy'][0]

-apache_unit = d.sentry.unit['apache2/0']
+apache_unit = d.sentry['apache2'][0]

-apache_unit2 = d.sentry.unit['apache2/1']
+apache_unit2 = d.sentry['apache2'][1]

The [0] bracket notation ensures you always get the 1st unit; similarly, [1] gives you the 2nd unit, no matter what the actual unit number may be.

I also added python-flake8 to your list of apt packages to make sure lint would be successful on systems without flake8.

Since these were such trivial changes, I made them while merging into the promulgated haproxy charm. The charm store should have an updated haproxy shortly that contains your changes + these tweaks.

Everything LGTM, so a big thanks to you for this proposal and for your patience during the review process!

review: Approve

« Back to merge proposal