[chef] Re: Re: Re: Re: Re: Re: Re: Re: Re: Re: Re: any plan for partial templates in Chef?

[Andrea Campi < >]

On Wed, Nov 14, 2012 at 11:34 PM, Daniel DeLeo < " target="_blank"> > wrote:

On Wednesday, November 14, 2012 at 1:52 PM, Andrea Campi wrote:

On Wed, Nov 14, 2012 at 3:29 AM, Daniel DeLeo < " target="_blank"> > wrote:

About the implementation, using the lambda as the partial finder is clever, but it'd be nice if it could be done with less magic.

On Wed, Nov 14, 2012 at 8:36 AM, Bryan Berry < " target="_blank"> > wrote:

echoing Dan's comment on the lambda, why not just make  partial_resolver  a method inside Chef::Mixin::Template::ChefContext next to your render method ?   Do you need the late binding that the lambda gives you?  

Chef::Mixin::Template::ChefContext is a "clean slate" that is very careful to expose as little data as possible to the template, so unfortunately that wouldn't work.

Chef::Mixin::Template is not possible either, since by design it doesn't know anything about nodes, run contexts and so on. That has value in unit testing and futher.

So a closure is a given, I think.

At minimum, you need to have a method defined in the template context and some way to pass the required state (run_context, default cookbook name) in to the template. Why not have something like TemplateContext = Struct.new(:run_context, :default_cookbook) that you pass in as an instance variable and have the render method access that?

Not a fan.

Either way, we absolutely do not want the user to do <%= partial_resolver %> And I don't think we want <%= @run_context %> either.

Adding more state to the TemplateContext would force us to then blacklist those. And suddenly our TemplateContext is not so clean after all…

 

Or a TemplateFinder class that takes those two bits of information in the constructor and will give you the path to the template when you call #find(template_relative_path) (and trigger the lazy template fetching if needed). The latter option also lets you keep the logic for template finding in one place, should it need to be changed later.

That's tempting, indeed. I will give that a try.

 

Yes, kinda. This is defining an API of sorts—it enables any caller of Chef::Mixin::Template::ChefContext to specify this partial_resolver, but they don't have to, it's up to them.

If I dropped the if and somebody invoked this "incorrectly", you would get something hard to debug.

If instead I replaced "foo" with something like:

raise "You cannot render a partial in this context"

at least you would have a clear hint of what went wrong.

But I have to say, this is pretty hypotetical: in the tree today we have exactly two uses for this mixin: Chef::Provider::Template, and specs.

Yeah, and the specs only use the template mixin to test it. If you're feeling ambitious, I think it makes sense to get rid of the mixin. For organizational purposes, I could see putting some code in lib/chef/provider/template/support_class.rb

Thanks again for taking this on. It's pretty clear from earlier in the thread that lots of people will be really happy to have this. 

My please :) I'm not sure I'll be able to find the time to get rid of the mixin, but I'll try to get a decent first implementation in good shape.