Charm Helpers

Merge lp://qastaging/~stub/charm-helpers/bug-1195649-fix-write-file into lp://qastaging/charm-helpers

bug-1195649-fix-write-file
Merge into devel

Proposed by Stuart Bishop on 2013-07-08

Status:

Merged

Merged at revision:

Proposed branch:

lp://qastaging/~stub/charm-helpers/bug-1195649-fix-write-file

Merge into:

lp://qastaging/charm-helpers

Diff against target:

203 lines (+68/-67)

4 files modified

charmhelpers/contrib/templating/pyformat.py (+13/-0)
charmhelpers/core/host.py (+6/-19)
tests/contrib/templating/test_pyformat.py (+36/-0)
tests/core/test_host.py (+13/-48)

To merge this branch:

bzr merge lp://qastaging/~stub/charm-helpers/bug-1195649-fix-write-file

Medium

Fix Released

Link a bug report

Reviewer	Review Type	Date Requested	Status
Matthew Wedgwood (community)		2013-07-08	Approve on 2013-07-10
Review via email: mp+173467@code.qastaging.launchpad.net

Description of the change

Move implicit rendering out of write_file and into a helper, per Bug #1195634.

Revision history for this message

Matthew Wedgwood (mew) wrote on 2013-07-08:

Stuart,

Thanks for cleaning this up. I agree that template rendering should be a separate concern from writing a file. I'm +1 on this change with one caveat:

As py_render() isn't host-related (nor is the existing render_template_file() function), it should live somewhere outside this module. charmhelpers.contrib.template might be appropriate, as most any other template renderers will have external dependencies.

-Matthew

review: Needs Fixing

lp://qastaging/~stub/charm-helpers/bug-1195649-fix-write-file updated on 2013-07-09

36. By Stuart Bishop on 2013-07-09: Move python templating to contrib per review feedback
37. By Stuart Bishop on 2013-07-09: Add missing test

Revision history for this message

Stuart Bishop (stub) wrote on 2013-07-09:

I've moved the helper to contrib.

I removed render_template_file() entirely for now. Perhaps write_file should do implicit rendering of usernames, groupnames & paths? It is magic, but at least with python formatting using curly brackets it is safe(ish) magic in these cases.

Revision history for this message

Matthew Wedgwood (mew) wrote on 2013-07-10:

I don't see a lot of value in having write_file do anything magic so long as there's an easy way to do it elsewhere. host.py, as I see it, should simply make it easy to interact with the host and shouldn't care much about the fact that it's running in a juju environment.

That means we should probably clean up rsync(), symlink(), and mkdir() as well. I'll file bugs for those.

This looks good!

review: Approve