Code review comment for lp:~abentley/charms/trusty/apache2/apache-website

Hi Aaron,

Thanks for your efforts to land this new functionality in the apache2 charm. I had the opportunity to review the merge proposal today, but ran into a few issues.

There was a merge conflict against trunk, so I used your branch directly.

One of the modules installed into .venv, linecache2, failed with a syntax error:

Compiling /charms/trusty/apache2/.venv/build/linecache2/linecache2/tests/inspect_fodder2.py ...
  File "/charms/trusty/apache2/.venv/build/linecache2/linecache2/tests/inspect_fodder2.py", line 102
    def keyworded(*arg1, arg2=1):

The install continued and the tests ran, however, two of them failed:

tests.test_cert.CertTests.test_is_selfsigned_cert_stale_private_address_changed
tests.test_cert.CertTests.test_is_selfsigned_cert_stale_public_address_changed

I don't believe your changes have anything to do with these two failures; they also fail in trunk. I'm not going to hold those failures against this merge. I've posted the complete log of the test run here:

http://pastebin.ubuntu.com/10622663/

I would like to see the interface documented in the README, so developers unfamiliar with the interface will know how to construct or use subordinates that take advantage of the new functionality, before I'm comfortable pushing this forward. With that, and the clean merge into trunk, this should be good to move forward.

Thanks again for your time on this!