Is there any plan to add Rails-style partials to Chef?
http://rdoc.info/docs/rails/3.2.8/ActionView/PartialRenderer
In my mind they would be incredibly useful, particularly for giant XML
configuration documents where 95% is boilerplate but the remaining 95% is
heavily customized for each application
+1 for this.
On Nov 11, 2012 1:02 PM, "Bryan Berry" bryan.berry@gmail.com wrote:
Is there any plan to add Rails-style partials to Chef?
http://rdoc.info/docs/rails/3.2.8/ActionView/PartialRendererIn my mind they would be incredibly useful, particularly for giant XML
configuration documents where 95% is boilerplate but the remaining 95% is
heavily customized for each application
even more telling would be a giant xml configuration document like that for
JBoss where I need to make add a parameter at the beginning and another at
the end. In between the beginning and end are 300 lines of boilerplate :/.
I would be able to DRY up that template by including the middle section as
a partial, resulting in a much more readable and maintainable template
On Sun, Nov 11, 2012 at 10:17 PM, Ranjib Dey dey.ranjib@gmail.com wrote:
+1 for this.
On Nov 11, 2012 1:02 PM, "Bryan Berry" bryan.berry@gmail.com wrote:Is there any plan to add Rails-style partials to Chef?
http://rdoc.info/docs/rails/3.2.8/ActionView/PartialRendererIn my mind they would be incredibly useful, particularly for giant XML
configuration documents where 95% is boilerplate but the remaining 95% is
heavily customized for each application
+1
On Sun, Nov 11, 2012 at 9:17 PM, Ranjib Dey dey.ranjib@gmail.com wrote:
+1 for this.
On Nov 11, 2012 1:02 PM, "Bryan Berry" bryan.berry@gmail.com wrote:
Is there any plan to add Rails-style partials to Chef?
http://rdoc.info/docs/rails/3.2.8/ActionView/PartialRendererIn my mind they would be incredibly useful, particularly for giant XML
configuration documents where 95% is boilerplate but the remaining 95% is
heavily customized for each application
--
Juanje
Hey Guys,
I actually opened up a ticket on this a while ago (
http://tickets.opscode.com/browse/CHEF-3249 ). This feature
would definitely make my life dealing a ton easier, XML FML! 
-John
On Sun, Nov 11, 2012 at 7:02 PM, Juanje Ojeda Croissier <
juanje.ojeda@gmail.com> wrote:
+1
On Sun, Nov 11, 2012 at 9:17 PM, Ranjib Dey dey.ranjib@gmail.com wrote:
+1 for this.
On Nov 11, 2012 1:02 PM, "Bryan Berry" bryan.berry@gmail.com wrote:
Is there any plan to add Rails-style partials to Chef?
http://rdoc.info/docs/rails/3.2.8/ActionView/PartialRendererIn my mind they would be incredibly useful, particularly for giant XML
configuration documents where 95% is boilerplate but the remaining 95%
is
heavily customized for each application--
JuanjeJuanje Ojeda - Las Palmas de Gran Canaria, España, Personal trainer | about.me
On Sunday, November 11, 2012 at 4:31 PM, John Dyer wrote:
Hey Guys,
I actually opened up a ticket on this a while ago ( http://tickets.opscode.com/browse/CHEF-3249 ). This feature would definitely make my life dealing a ton easier, XML FML!
-John
On Sun, Nov 11, 2012 at 7:02 PM, Juanje Ojeda Croissier <juanje.ojeda@gmail.com (mailto:juanje.ojeda@gmail.com)> wrote:
+1
On Sun, Nov 11, 2012 at 9:17 PM, Ranjib Dey <dey.ranjib@gmail.com (mailto:dey.ranjib@gmail.com)> wrote:
+1 for this.
On Nov 11, 2012 1:02 PM, "Bryan Berry" <bryan.berry@gmail.com (mailto:bryan.berry@gmail.com)> wrote:
Is there any plan to add Rails-style partials to Chef?
http://rdoc.info/docs/rails/3.2.8/ActionView/PartialRendererIn my mind they would be incredibly useful, particularly for giant XML
configuration documents where 95% is boilerplate but the remaining 95% is
heavily customized for each application--
JuanjeJuanje Ojeda - Las Palmas de Gran Canaria, España, Personal trainer | about.me
I don't think Opscode will be able to get to this soon, but we'd definitely welcome a patch for this. The only tricky bit is that you need to factor out the logic for finding and downloading templates so that you can use it in the partial rendering code. The relevant bits are here: chef/lib/chef/provider/template.rb at main · chef/chef · GitHub
--
Daniel DeLeo
Hey Daniel,
nice to know that it is on your radar. I will take a look at it and see
what I can do
On Mon, Nov 12, 2012 at 6:22 PM, Daniel DeLeo dan@kallistec.com wrote:
On Sunday, November 11, 2012 at 4:31 PM, John Dyer wrote:
Hey Guys,
I actually opened up a ticket on this a while ago (
http://tickets.opscode.com/browse/CHEF-3249 ). This feature
would definitely make my life dealing a ton easier, XML FML!-John
On Sun, Nov 11, 2012 at 7:02 PM, Juanje Ojeda Croissier <
juanje.ojeda@gmail.com> wrote:+1
On Sun, Nov 11, 2012 at 9:17 PM, Ranjib Dey dey.ranjib@gmail.com wrote:
+1 for this.
On Nov 11, 2012 1:02 PM, "Bryan Berry" bryan.berry@gmail.com wrote:
Is there any plan to add Rails-style partials to Chef?
http://rdoc.info/docs/rails/3.2.8/ActionView/PartialRendererIn my mind they would be incredibly useful, particularly for giant XML
configuration documents where 95% is boilerplate but the remaining 95%
is
heavily customized for each application--
JuanjeJuanje Ojeda - Las Palmas de Gran Canaria, España, Personal trainer | about.me
I don't think Opscode will be able to get to this soon, but we'd
definitely welcome a patch for this. The only tricky bit is that you need
to factor out the logic for finding and downloading templates so that you
can use it in the partial rendering code. The relevant bits are here:
chef/lib/chef/provider/template.rb at main · chef/chef · GitHub--
Daniel DeLeo
Just for kicks, I spent an hour tonight trying to figure out how
complicated this would be.
This is just a proof of concept, but the spec is green.
It would probably another 1-2 hours to prepare a proper pull request.
Unless this is deemed too crazy, of course 
On Tuesday, November 13, 2012 at 4:26 PM, Andrea Campi wrote:
Just for kicks, I spent an hour tonight trying to figure out how complicated this would be.
This is just a proof of concept, but the spec is green.
It would probably another 1-2 hours to prepare a proper pull request. Unless this is deemed too crazy, of course
Attachments:
- 0001-Quick-and-dirty-implementation-of-render-partial-in-.patch
Overall it's the right direction.
Some questions:
Is there value in having the #render method take either a Hash or String to select which partial to render? Why not just have the method signature be: render(partial_name, variables_as_a_hash=nil)? If variables_as_a_hash was given, you replace the current context variables with a the specified set, otherwise keep them the same.
Following from the above, is there any need to support running partials from different cookbooks than the template that's being rendered? Or from an on-disk (local true) template? This would lead you to a method/option signature like
render("partial_name", :variables => {:foo => "bar"}, :cookbook => "other-cookbook", :local => true, :source => "/var/mytemplates/partial.conf.erb")
Is there value in the leading underscore in partial names? This will be natural for Rails people, and yet another thing to learn for everyone else. Does it make it easier to look at your cookbooks and figure out what's going on if the partials look different?
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.
--
Daniel DeLeo
this is super exciting!
I do not have any rails experience and I personally find the
"_#{partial_name}" pretty intuitive. It says to me that the template
doesn't exist independent of its parent. Also, I see partials as becoming
an advanced feature that n00bs wouldn't find or need until they are farther
along.
I myself am pretty new to the chef codebase and to doing more Ruby than
just Chef and having a little trouble parsing this code:
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?
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
Sorry if my questions are ignorant, i am a still a total n00b when it comes
to understanding complex ruby libs
On Wed, Nov 14, 2012 at 3:29 AM, Daniel DeLeo dan@kallistec.com wrote:
On Tuesday, November 13, 2012 at 4:26 PM, Andrea Campi wrote:
Just for kicks, I spent an hour tonight trying to figure out how
complicated this would be.This is just a proof of concept, but the spec is green.
It would probably another 1-2 hours to prepare a proper pull request.
Unless this is deemed too crazy, of courseAttachments:
- 0001-Quick-and-dirty-implementation-of-render-partial-in-.patch
Overall it's the right direction.
Some questions:
Is there value in having the #render method take either a Hash or String
to select which partial to render? Why not just have the method signature
be: render(partial_name, variables_as_a_hash=nil)? If variables_as_a_hash
was given, you replace the current context variables with a the specified
set, otherwise keep them the same.Following from the above, is there any need to support running partials
from different cookbooks than the template that's being rendered? Or from
an on-disk (local true) template? This would lead you to a method/option
signature like
render("partial_name", :variables => {:foo => "bar"}, :cookbook =>
"other-cookbook", :local => true, :source =>
"/var/mytemplates/partial.conf.erb")Is there value in the leading underscore in partial names? This will be
natural for Rails people, and yet another thing to learn for everyone else.
Does it make it easier to look at your cookbooks and figure out what's
going on if the partials look different?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.--
Daniel DeLeo
On Tuesday, November 13, 2012 at 11:36 PM, Bryan Berry wrote:
this is super exciting!
I do not have any rails experience and I personally find the "_#{partial_name}" pretty intuitive. It says to me that the template doesn't exist independent of its parent. Also, I see partials as becoming an advanced feature that n00bs wouldn't find or need until they are farther along.
Thanks for sharing your opinions. The various ways that Chef encodes information in strings, filenames, etc. worries me, but as an expert I don't have any way to objectively gauge the impact on a less experienced user.
One question, though: is there a use case where you would want a template to be both a partial and a non-partial? Perhaps you have one system that supports "conf.d/" style config and another that doesn't? I suppose that could be resolved by setting the source parameter in the template resource to "_ususally_a_partial.conf.erb", however.
Any thoughts on the method signature?
--
Daniel DeLeo
hard for me to say if I might use a partial document as a standalone
document in other circumstances .
the circumstances where I personally would use a partial
haproxy.cfg
jboss ginormous xml configs
tomcat server.xml, context.xml, web.xml
only w/ tomcat is there the possibility that some of the "fragments" could
stand alone but I am not sure. Even then, a standalone fragment would need
the extra decorator
to actually exist on its own
I think I prefer kallistec's method signature, however I would also want
the ability to see all the variables passed to the parent template unless
intentionally overwritten
On Wed, Nov 14, 2012 at 6:20 PM, Daniel DeLeo dan@kallistec.com wrote:
On Tuesday, November 13, 2012 at 11:36 PM, Bryan Berry wrote:
this is super exciting!
I do not have any rails experience and I personally find the
"_#{partial_name}" pretty intuitive. It says to me that the template
doesn't exist independent of its parent. Also, I see partials as becoming
an advanced feature that n00bs wouldn't find or need until they are farther
along.Thanks for sharing your opinions. The various ways that Chef encodes
information in strings, filenames, etc. worries me, but as an expert I
don't have any way to objectively gauge the impact on a less experienced
user.One question, though: is there a use case where you would want a template
to be both a partial and a non-partial? Perhaps you have one system that
supports "conf.d/" style config and another that doesn't? I suppose that
could be resolved by setting the source parameter in the template resource
to "_ususally_a_partial.conf.erb", however.Any thoughts on the method signature?
--
Daniel DeLeo
On Wed, Nov 14, 2012 at 3:29 AM, Daniel DeLeo dan@kallistec.com wrote:
On Tuesday, November 13, 2012 at 4:26 PM, Andrea Campi wrote:
Just for kicks, I spent an hour tonight trying to figure out how
complicated this would be.This is just a proof of concept, but the spec is green.
It would probably another 1-2 hours to prepare a proper pull request.
Unless this is deemed too crazy, of courseAttachments:
- 0001-Quick-and-dirty-implementation-of-render-partial-in-.patch
Overall it's the right direction.
Some questions:
- Is there value in having the #render method take either a Hash or String
to select which partial to render? Why not just have the method signature
be: render(partial_name, variables_as_a_hash=nil)? If variables_as_a_hash
was given, you replace the current context variables with a the specified
set, otherwise keep them the same.
No other reason that a Rails mindset.
Looking at your example below, I think having the first argument be a
String would make the most sense from a Chef background.
- Following from the above, is there any need to support running partials
from different cookbooks than the template that's being rendered? Or from
an on-disk (local true) template? This would lead you to a method/option
signature like
render("partial_name", :variables => {:foo => "bar"}, :cookbook =>
"other-cookbook", :local => true, :source =>
"/var/mytemplates/partial.conf.erb")
Here too, the Rails person in me cringes a little at :local => true, but
the Chef person is in strong agreement.
As for use cases, I think we want:
partials in the same cookbook, to keep things manageable; these are easy;
partials in different cookbooks: yes please!
There are LWRPs out there that generate a template and let you specify the
name of the cookbook. So if you want to override them, you end up copying
them from the original cookbook to the one that uses the LWRP—all just to
change 2 lines.
Imagine being able to say:
<%= render "foo.conf.erb, :cookbook => "foo" %>
Set bar=baz # overrides what's in the original template
$ cat etc-hosts.erb
<% hosts.each do |h| %>…<% end %>
<%= render "/etc/hosts.local %>
$ cat master.conf.erb
<%= render "vhost.erb", :collection => @vhosts, :as => :vhost %>
Where vhost.erb would be rendered once per element in @vhosts (that must be
an Enumerable), which would be passed as @vhost
Looking at your proposal, I'm not sure about the advantage of passing
:source except regularity when compared to a Chef template resource.
Care to elaborate?
- Is there value in the leading underscore in partial names? This will be
natural for Rails people, and yet another thing to learn for everyone else.
Does it make it easier to look at your cookbooks and figure out what's
going on if the partials look different?
On further thought, no, I think it's too much.
Personally I find it useful to have a naming convention that tells me that
a template is meant to be used as a partial.
But I don't think that warrants showing down people's throats—let them
figure out their own naming convention.
On Wed, Nov 14, 2012 at 6:20 PM, Daniel DeLeo dan@kallistec.com wrote:
One question, though: is there a use case where you would want a template
to be both a partial and a non-partial? Perhaps you have one system that
supports "conf.d/" style config and another that doesn't?
That's a good use case I think. I'm all in favor of removing the leading
underscore.
On Wed, Nov 14, 2012 at 3:29 AM, Daniel DeLeo dan@kallistec.com 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 bryan.berry@gmail.com 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.
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_resolvertemplate_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.
Gosh I am embarrassed that I didn't notice kallistec's note on conf.d/
files. I was totally consumed with the needs of enterprise java apps 
On Nov 14, 2012 10:52 PM, "Andrea Campi" andrea.campi@zephirworks.com
wrote:
On Wed, Nov 14, 2012 at 3:29 AM, Daniel DeLeo dan@kallistec.com 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 bryan.berry@gmail.comwrote:
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.
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_resolvertemplate_location = @partial_resolver.call(partial_name)eruby = Erubis::Eruby.new(IO.read(template_location))output = eruby.evaluate(self)else"foo"endYes, 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.
All of this looks better than my nginx hacks to provide mini-templates in
the default attributes:
attributes/default.rb:
default[:nginx][:knewton_common_nginx_client_opts] = proc { |n|
r <<EBLOCK
client_body_buffer_size 1m;
client_header_buffer_size 1k;
client_max_body_size #{n[:nginx][:client_max_body_size]};
large_client_header_buffers 4 8k;
EBLOCK
}
recipes/my_nginx.rb:
template "ordered_include.conf" do
path "#{node[:nginx][:dir]}/ordered_include.conf"
source "ordered_include.conf.erb"
owner "myuser"
group "mygroup"
mode "0644"
variables({
:chef_warning_header =>
node[:nginx][:chef_warning_header].call(s_node), # uses cluster, needs
s_node
:my_common_nginx_client_opts =>
node[:nginx][:knewton_common_nginx_client_opts].call(node
), # uses :nginx, needs node.
:my_common_gzip_opts =>
node[:nginx][:my_common_gzip_opts].call(node),
:https_common_proxy_opts =>
node[:nginx][:https_common_proxy_opts].call(node),
:http_common_proxy_opts =>
node[:nginx][:http_common_proxy_opts].call(node),
:upstream_servers => upstream_servers,
:enabled_sites_conf => enabled_configs,
:sites_available_dir =>
node[:nginx][:sites_available],
})
end
etc.
So how will the proposal to introduce partials make it clearer what's
happening (clearer than this, anyway) to the new chef-user who just wants
to download a cookbook and have it work? And break in a clearer manner?
-Peter
On Wed, Nov 14, 2012 at 4:57 PM, Bryan Berry bryan.berry@gmail.com wrote:
Gosh I am embarrassed that I didn't notice kallistec's note on conf.d/
files. I was totally consumed with the needs of enterprise java apps
On Nov 14, 2012 10:52 PM, "Andrea Campi" andrea.campi@zephirworks.com
wrote:On Wed, Nov 14, 2012 at 3:29 AM, Daniel DeLeo dan@kallistec.com 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 bryan.berry@gmail.comwrote:
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.
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_resolvertemplate_location = @partial_resolver.call(partial_name)eruby = Erubis::Eruby.new(IO.read(template_location))output = eruby.evaluate(self)else"foo"endYes, 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.
On Wednesday, November 14, 2012 at 1:52 PM, Andrea Campi wrote:
On Wed, Nov 14, 2012 at 3:29 AM, Daniel DeLeo <dan@kallistec.com (mailto:dan@kallistec.com)> 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 <bryan.berry@gmail.com (mailto:bryan.berry@gmail.com)> 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_resolvertemplate_location = @partial_resolver.call(partial_name)eruby = Erubis::Eruby.new(IO.read(template_location))output = eruby.evaluate(self)else"foo"endYes, 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
On Wednesday, November 14, 2012 at 1:38 PM, Andrea Campi wrote:
On Wed, Nov 14, 2012 at 3:29 AM, Daniel DeLeo <dan@kallistec.com (mailto:dan@kallistec.com)> wrote:
On Tuesday, November 13, 2012 at 4:26 PM, Andrea Campi wrote:
Just for kicks, I spent an hour tonight trying to figure out how complicated this would be.
This is just a proof of concept, but the spec is green.
It would probably another 1-2 hours to prepare a proper pull request. Unless this is deemed too crazy, of course
Attachments:
- 0001-Quick-and-dirty-implementation-of-render-partial-in-.patch
Overall it's the right direction.
Some questions:
- Is there value in having the #render method take either a Hash or String to select which partial to render? Why not just have the method signature be: render(partial_name, variables_as_a_hash=nil)? If variables_as_a_hash was given, you replace the current context variables with a the specified set, otherwise keep them the same.
No other reason that a Rails mindset.
Looking at your example below, I think having the first argument be a String would make the most sense from a Chef background.
- Following from the above, is there any need to support running partials from different cookbooks than the template that's being rendered? Or from an on-disk (
local true) template? This would lead you to a method/option signature like
render("partial_name", :variables => {:foo => "bar"}, :cookbook => "other-cookbook", :local => true, :source => "/var/mytemplates/partial.conf.erb")Here too, the Rails person in me cringes a little at :local => true, but the Chef person is in strong agreement.
As for use cases, I think we want:
partials in the same cookbook, to keep things manageable; these are easy;
partials in different cookbooks: yes please!
There are LWRPs out there that generate a template and let you specify the name of the cookbook. So if you want to override them, you end up copying them from the original cookbook to the one that uses the LWRP—all just to change 2 lines.
Imagine being able to say:<%= render "foo.conf.erb, :cookbook => "foo" %>
Set bar=baz # overrides what's in the original template
- local partials: totally!
$ cat etc-hosts.erb
<% hosts.each do |h| %>…<% end %>
<%= render "/etc/hosts.local %>
- (bonus points) collection partials:
$ cat master.conf.erb
<%= render "vhost.erb", :collection => @vhosts, :as => :vhost %>Where vhost.erb would be rendered once per element in @vhosts (that must be an Enumerable), which would be passed as @vhost
Yeah, that would be a nice addition.
Looking at your proposal, I'm not sure about the advantage of passing :source except regularity when compared to a Chef template resource.
Care to elaborate?
I was looking at the list of options for the template resource and adding them all in to my example. I only see this making any sense at all when using local templates, so you could write something like:
render "app specific config stanza",
:local => true,
:path => "/srv/my_app/current/config_stanza.erb"
Maybe that's more readable for some people? I don't have a strong opinion here.
- Is there value in the leading underscore in partial names? This will be natural for Rails people, and yet another thing to learn for everyone else. Does it make it easier to look at your cookbooks and figure out what's going on if the partials look different?
On further thought, no, I think it's too much.
Personally I find it useful to have a naming convention that tells me that a template is meant to be used as a partial.
But I don't think that warrants showing down people's throats—let them figure out their own naming convention.
I could argue both sides on this, I just wanted to make sure we thought about it.
--
Daniel DeLeo
On Wed, Nov 14, 2012 at 11:34 PM, Daniel DeLeo dan@kallistec.com wrote:
On Wednesday, November 14, 2012 at 1:52 PM, Andrea Campi wrote:
On Wed, Nov 14, 2012 at 3:29 AM, Daniel DeLeo dan@kallistec.com 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 bryan.berry@gmail.comwrote:
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.rbThanks 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.