Comment 1 for bug 1553534

Revision history for this message
Cory Johns (johnsca) wrote :

Joe,

Thank you for your submission to the charm ecosystem! This charm looks great, and I have only a few minor suggestions for improvement.

First, it would be very helpful to use status-set[1] to report blocking conditions, such as missing the required hostname config or missing the relation to postgresql, as well as informing the admin of the progress of deployment or restart. For example, when you detect that the hostname is not set, you can use this to report that information to the admin:

    status-set blocked "Please set the hostname config option"

Second, it would be nice to add an action[2] for upgrade-charm, so that it can be run directly instead of relying on `juju run`.

Last, you note in the README and config.yaml that hostname cannot be changed after deployment. Though we generally frown very much on what we call "immutable config," I understand that this is a limitation from the diaspora* software itself due to the hostname getting persisted in the database. However, there is nothing in the charm code that actually prevents the hostname from being changed after db initialization. Perhaps some checks would be warranted, or at least more explanatory text in the README and config.yaml as to the consequences of changing that value would be useful.

[1]: https://jujucharms.com/docs/stable/reference-hook-tools#status-set
[2]: https://jujucharms.com/docs/stable/authors-charm-actions