Code review comment for lp://staging/~celebdor/charms/precise/cassandra/hostname_resolve

Revision history for this message
Matt Bruzek (mbruzek) wrote :

Hello Antoni,

Thank you for submitting this fix. It is my understanding this fix allows cassandra to be installed on LXC.

The code adds a configuration option that is only evaluated at install time. If the customer were to change the package_version after install the operation would have no effect. This is what we call "immutable configuration". This is because "config-changed" is the only hook called when the Juju user changes the configuration in the GUI or on the command line.

If you are going to add a configuration option like this I would like to see the configuration option read and reacted in the "configure_cassandra" function of the hooks/cassandra-common file.

You can keep the code in the "install" function but need to add it to something that will be called on "config-changed".

Please let me know when you are ready for another review.

Thanks!

review: Needs Fixing

« Back to merge proposal