Code review comment for lp:~sinzui/charmworld/heartbeat

Revision history for this message
Richard Harding (rharding) wrote :

A couple of notes. First, thanks for this. I think it's a great step.

This actually has me wondering if we can't move the logic into a charmworld/lib/health.py and keep the logic of adding/checking there make it more reusable. I'm actually thinking I'd love to provide a cli tool to sanity check your charmworld install for devs as well.

In the failing cases where we have on charm, for #IS I'd think that's a failing condition. So in #97 and the other checks like it should that set the status to fail?

Finally, if we're going to FAIL for no featured charms, should we adjust our bootstrap to auto select at least one charm to be featured automatically. This would help with the 'new dev setup' story as well as they could be sure to have featured already and be able to point the GUI at the dev instance without issue.

Sorry, this grew.

tl;dr
1. Looks good!
2. I think it'd be nice if the health checks were a module vs in the view.
3. I think we should auto select a single featured charm during install/setup?
4. Should 0 charms be failing for IS purposes?

review: Approve

« Back to merge proposal