Merge lp:~johnsca/charm-helpers/charm-helpers into lp:~cf-charmers/charm-helpers/cloud-foundry

[Cory Johns]

Refactor services & templating framework to core

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

[Cory Johns (johnsca)]

Reviewers: mp+219910_code.launchpad.net,

Message:
Please take a look.

Description:
Refactor services & templating framework to core

https://code.launchpad.net/~johnsca/charm-helpers/charm-helpers/+merge/219910

(do not edit description out of merge proposal)

Please review this at https://codereview.appspot.com/92420046/

Affected files (+1083, -852 lines):
   A [revision details]
   M charmhelpers/contrib/cloudfoundry/azure/add-cf-ports.py
   M charmhelpers/contrib/cloudfoundry/common.py
   D charmhelpers/contrib/cloudfoundry/config_helper.py
   M charmhelpers/contrib/cloudfoundry/contexts.py
   D charmhelpers/contrib/cloudfoundry/install.py
   D charmhelpers/contrib/cloudfoundry/services.py
   D charmhelpers/contrib/cloudfoundry/upstart_helper.py
   M charmhelpers/core/host.py
   A charmhelpers/core/services.py
   A charmhelpers/core/templating.py
   D tests/contrib/cloudfoundry/templates/cloud_controller_ng.yml
   D tests/contrib/cloudfoundry/templates/fake_cc.yml
   D tests/contrib/cloudfoundry/templates/nginx.conf
   D tests/contrib/cloudfoundry/templates/test.conf
   D tests/contrib/cloudfoundry/test_config_helper.py
   D tests/contrib/cloudfoundry/test_install.py
   M tests/contrib/cloudfoundry/test_render_context.py
   D tests/contrib/cloudfoundry/test_services.py
   A tests/core/templates/cloud_controller_ng.yml
   A tests/core/templates/fake_cc.yml
   A tests/core/templates/nginx.conf
   A tests/core/templates/test.conf
   M tests/core/test_host.py
   A tests/core/test_services.py
   A tests/core/test_templating.py

[Cory Johns (johnsca)]

Please take a look.

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

[Cory Johns (johnsca)]

Please take a look.

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

[Cory Johns (johnsca)]

Please take a look.

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

[Benjamin Saller (bcsaller)]

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/

[Cory Johns (johnsca)]

*** Submitted:

Refactor services & templating framework to core

(Forgot to use lbox on this one; my bad.)

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