Code review comment for lp:~jose/charms/precise/tracks/fixes

José,

Thanks so much for these improvements! This charm is so much more robust and secure for these changes!

However, I noticed an issue regarding the port handling: if you set the port to a small number initially, a log message indicating that the port is not allowed is recorded, but the upstart script is still created with the low-numbered port. Further, if you set the port to a high-numbered port and then later change the port number, because the .portnotsupported file doesn't exist, the the port check is skipped entirely and the port is not changed.

I think that the .portnotsupported file check can be removed entirely, and the port check in the install script should be moved down to wrap the creation of the upstart conf, with an else clause to remove the conf if it exists. The existing service will also need to be stopped before the upstart conf is changed or removed and restarted after the new conf is in place (though, there's also the database to consider, so there might need to be a .db-configured marker file or something, that is checked before starting the service from hooks/install).

Additionally, to allow this to work with a reverse proxy, as is the intention, I believe the website-relation-joined hook must be added, but I think it can be a simple boiler-plate hook, such as: http://pastebin.ubuntu.com/7412868/

Lastly, I had a question regarding the db-relation-changed hook: do you know if it matters if rake db:migrate is called multiple times? If it does, then that hook isn't idempotent and needs a check, but since the original hook doesn't have that, maybe it doesn't matter if it's called again.