Comment 2 for bug 1083008

Revision history for this message
Juan L. Negron (negronjl) wrote :

Hi Paul:

Here is my feedback on auth-proxy:
- install
--- As a subordinate, you should be careful when replacing files that could also be used by the main charm. I see that you are replacing default and default-ssl which may already have been modified and used by the main charm. At a minimum, you should determine if the file has been modified before replacing it. Arbitrarily replacing such a common ( and maybe essential ) file could be dangerous.

- stop
--- I see a similar problem with stopping apache2 altogether here ... As a subordinate charm, you don't know if apache is already being used by the main charm. Stopping it could stop the main charm as well.

- config-changed
--- I also see some of that here as well ....

All in all, I believe that the changes applied in this charm have to be applied very carefully and should be very targeted. A possible option would be to not assume that default and default-ssl are being used and allow the user to specify the name of the site being used ( you could default to default and default-ssl ). This could give you some indication of the site being used by the main charm.

I am marking the charm as Incomplete while you address the issues mentioned above. I would be happy to review the charm again when you're ready.

Please feel free to reach out to any of us in #juju on freenet ... we are here to help in any way we can.

Thanks,

Juan