Comment 6 for bug 886365

Hi Paul,

This is by no means an exhaustive review, but I did have a few comments:

- I'm not sure of the advantage of cat-ing files directly from the install hook rather than including them in a files directory and then copying them - this way you have a separation of content from logic (/etc/apache2/httpd.conf, /opt/graphite/conf/graphite.wsgi,/opt/graphite/webapp/graphite/local_settings.py).
- Since you're using apache, maybe it'd be worth considering doing this as a subordinate to the apache2 charm so that you get all of the advantages of using that charm (performance tuning of apache itself, ability to use SSL certs, etc.).
- It'd be nice to have other options for installing graphite than downloading directly from github.com, doing pip installs - in many corporate environments the place you're deploying from will have egress firewalls that may prevent access to these. If someone has a package location or other means of installing graphite it'd be good to support this.
- It seems odd to have the following comment in the configuration file you're creating:
# XXX In order for the django admin site media to work you
# must change @DJANGO_ROOT@ to be the path to your django
# installation, which is probably something like:
- There seems to be an empty appamor directory

Thanks, Tom