cf-charmers team mailing list archive

[Benjamin Saller <benjamin.saller@xxxxxxxxxxxxx>]

LGTM w/minors

While not a huge issue, I really would like something better done with
the chownr method. Still ok with a shell out there.
Thanks for this, feel free to land even if all you decide to do here is
change Exception to OSError, but if you'd like me to look at the change
again, happy to.


https://codereview.appspot.com/92420046/diff/60001/charmhelpers/contrib/cloudfoundry/contexts.py
File charmhelpers/contrib/cloudfoundry/contexts.py (right):

https://codereview.appspot.com/92420046/diff/60001/charmhelpers/contrib/cloudfoundry/contexts.py#newcode38
charmhelpers/contrib/cloudfoundry/contexts.py:38: ctx['db']['port'] =
'3306'
s/'db'/self.interface/ on both lines

https://codereview.appspot.com/92420046/diff/60001/charmhelpers/core/host.py
File charmhelpers/core/host.py (right):

https://codereview.appspot.com/92420046/diff/60001/charmhelpers/core/host.py#newcode333
charmhelpers/core/host.py:333: if fatal:
This means that something could go wrong and we continue blindly. I
don't think this is what we want. If nothing else we'd only want to
catch errors related to things like symbolic links here and not things
like the path doesn't exist (maybe an upstart job is modifying the
filesytem as well for example) which Exception will catch.

https://codereview.appspot.com/92420046/diff/60001/charmhelpers/core/services.py
File charmhelpers/core/services.py (right):

https://codereview.appspot.com/92420046/diff/60001/charmhelpers/core/services.py#newcode53
charmhelpers/core/services.py:53: manager_type = service.get('type',
UpstartService)
Thanks for adding this, I think this is a good solution.

https://codereview.appspot.com/92420046/diff/60001/charmhelpers/core/templating.py
File charmhelpers/core/templating.py (right):

https://codereview.appspot.com/92420046/diff/60001/charmhelpers/core/templating.py#newcode7
charmhelpers/core/templating.py:7: LOADER = None
No longer used? I think we can remove it

https://codereview.appspot.com/92420046/

-- 
https://code.launchpad.net/~johnsca/charm-helpers/charm-helpers/+merge/219910
Your team Cloud Foundry Charmers is requested to review the proposed merge of lp:~johnsca/charm-helpers/charm-helpers into lp:~cf-charmers/charm-helpers/cloud-foundry.