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/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.
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 contrib/ cloudfoundry/ contexts. py (right):
File charmhelpers/
https:/ /codereview. appspot. com/92420046/ diff/60001/ charmhelpers/ contrib/ cloudfoundry/ contexts. py#newcode38 contrib/ cloudfoundry/ contexts. py:38: ctx['db']['port'] = self.interface/ on both lines
charmhelpers/
'3306'
s/'db'/
https:/ /codereview. appspot. com/92420046/ diff/60001/ charmhelpers/ core/host. py core/host. py (right):
File charmhelpers/
https:/ /codereview. appspot. com/92420046/ diff/60001/ charmhelpers/ core/host. py#newcode333 core/host. py:333: if fatal:
charmhelpers/
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 core/services. py (right):
File charmhelpers/
https:/ /codereview. appspot. com/92420046/ diff/60001/ charmhelpers/ core/services. py#newcode53 core/services. py:53: manager_type = service.get('type',
charmhelpers/
UpstartService)
Thanks for adding this, I think this is a good solution.
https:/ /codereview. appspot. com/92420046/ diff/60001/ charmhelpers/ core/templating .py core/templating .py (right):
File charmhelpers/
https:/ /codereview. appspot. com/92420046/ diff/60001/ charmhelpers/ core/templating .py#newcode7 core/templating .py:7: LOADER = None
charmhelpers/
No longer used? I think we can remove it
https:/ /codereview. appspot. com/92420046/