On Jul 2, 2012, at 1:59 PM, Brice Figureau wrote:
> Hi,
>
> While wrapping up my compiler performance patch (soon to be released), I
> stumbled on this test (in compiler_spec.rb):
>
> it "should fail to add resources that conflict with existing resources" do
> path = make_absolute("/foo")
> file1 = Puppet::Type.type(:file).new :path => path
> file2 = Puppet::Type.type(:file).new :path => path
>
> @compiler.add_resource(@scope, file1)
> lambda { @compiler.add_resource(@scope, file2) }.should
> raise_error(Puppet::Resource::Catalog::DuplicateResourceError)
> end
>
> I'm wondering what's the point in adding already compiled RAL resources
> to a compiler, especially when those are even not inheriting from
> Puppet::Resource.
>
> The reason I'm asking this is that I added two methods to
> Puppet::Resource (namely class? and stage?) to prevent the DRY smell of
> doing:
> resource.type.to_s.downcase == "stage" (and equivalent for "class")
>
> My intent was to reduce the number of times this code structure is
> executed during a compilation (mainly to prevent creation of so many
> short-lived String objects).
>
> Did I miss anything obvious?
Probably not. This is a pretty hideous aspect of the system right now, and
until we can merge Puppet::Type and Puppet::Resource, it's probably going to be
confusing to everyone who isn't me (it's my fault, which is why it's not
confusing to me).
Currently, Puppet::Type and Puppet::Resource are entirely separate class
hierarchies, although I would like the resource-instance behaviors of
Puppet::Type to be merged into Puppet::Resource at some point.
In general, and *almost* certainly for the cases of Catalog, they behave
identically. I expect that what's happening here is that it doesn't much
matter what kind of resource we add, the failure should be the same. It sounds
like you think we should be adding Puppet::Resource instances, which is
probably right, but maybe this code is older, from before that class existed or
something.
This test is important because (I think) Catalog is used for the test at
compile time, so you're right it would make more sense to use Puppet::Resource
here, rather than Puppet::Type.
Does that help?
--
Luke Kanies | http://about.me/lak | http://puppetlabs.com/ | +1-615-594-8199
--
You received this message because you are subscribed to the Google Groups
"Puppet Developers" group.
To post to this group, send email to [email protected].
To unsubscribe from this group, send email to
[email protected].
For more options, visit this group at
http://groups.google.com/group/puppet-dev?hl=en.
