Comment 2 for bug 908308

Doing the review in a couple of steps for the sake of time right now:
1. static review
2. promulgate
3. dynamic review in the context of a full openstack deployment

---

Static review:

- readme: Please add a readme file... the charm browser displays them automatically now. Do whatever makes sense for readme files in the overall context of the set of openstack charms.

- upgrade: What's the upgrade path look like for this charm? There're lots of moving pieces in openstack as a whole... is it even possible? If so, please add an `upgrade-charm` hook/link... if not, please note this in readme.

---

discussion/recommendations:

- [eventually] definitely split utils.py out into some charm-helpers-python tools

- [eventually] maybe extract your 'valid_service' stuff out into charm-helpers too... that'd be generally useful

- [wishlist] I like your config_get wrapper... I'd love to eventually see one that also allowed for locally persistent config like facter... all hidden behind one config interface. Maybe it's easier to just extend juju to handle `config-set` from a hook... dunno

- stylistically, it's easier to look at only hooks in hooks/... everything else included/linked from something like $CHARM_DIR/lib... no biggie obviously

- really like the debug/verbose options... this is a note to self to recommend that as a best-practice for large/complex/fragile services across the board

---

Please go ahead and promulgate... looks good!

I'll get your help to do tests in a full-stack context next week... filing bugs against the promulgated charm with any problems.