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

[Daniel DeLeo < >]

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? 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.

 

What I plan to do to make this more readable is to turn the body of the lambda into a proper method of Chef::Provider::Template. We will still need to pass a proc / lambda into the options hash, but it will be less magic.

also, why do you have an if statement in the following, do you expect the partial_resolver to be pluggable or somehow converted to nil ? 

+          if @partial_resolver

+            template_location = @partial_resolver.call(partial_name)

+            eruby = Erubis::Eruby.new(IO.read(template_location))

+            output = eruby.evaluate(self)

+          else

+            "foo"

+          end

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. 

-- 

Daniel DeLeo