Code review comment for lp://staging/~bjornt/charms/precise/landscape-client/no-landscape-config-call

Revision history for this message
Thomas Herve (therve) wrote :

Looks great! Thanks for starting to factor common code.

[1] config-changed and juju-info-relation-joined share lots of code, it'd be nice to add an additional function for those.

[2] I don't think we should exit_with_error on register error: if the server is out for a bit for example, the charm shouldn't not error out, and just retry to register later on. OTOH, I guess it's a problem if you're missing a configuration parameter. I suspect we want to re-register anyway if change the config later on? IE, instead of having the on_error call, I would stop_client_and_disable_init_script() on config-changed.

Thanks!

review: Approve

« Back to merge proposal