Thinking about it I am of the opinion it should revert to include and  
warn the user. That way people get the class but understand why the  
dependency might not have worked. But I understand why that might also  
be counter-intuitive to some and cause confusion.

Others?

P.S. Excuse the top post - iPhone.

Regards

James Turnbull
http://www.james-turnbull.net

On 20/09/2009, at 2:58 PM, Luke Kanies <[email protected]> wrote:

>
> I couldn't find a way to make it compatible with
> earlier clients, so the docs specify that
> it doesn't work with them, and it helpfully fails.
>
> Signed-off-by: Luke Kanies <[email protected]>
> ---
> lib/puppet/parser/functions/require.rb       |   16 ++++++++++++++--
> lib/puppet/parser/resource.rb                |   10 +++++-----
> spec/integration/parser/functions/require.rb |    8 +++++---
> spec/unit/parser/functions/require.rb        |   18 ++++++++++--------
> 4 files changed, 34 insertions(+), 18 deletions(-)
>
> diff --git a/lib/puppet/parser/functions/require.rb b/lib/puppet/ 
> parser/functions/require.rb
> index 7d73831..dda8cf0 100644
> --- a/lib/puppet/parser/functions/require.rb
> +++ b/lib/puppet/parser/functions/require.rb
> @@ -23,12 +23,24 @@ class depends on the required class.
>        file { '/foo': notify => Service[foo] }
>     }
>
> +Note that this function only works with clients 0.25 and later, and  
> it will
> +fail if used with earlier clients.
> +
> ") do |vals|
> +        if resource.metaparam_compatibility_mode?
> +            raise Puppet::ParseError, "The 'require' function is  
> only compatible with clients at 0.25 and above"
> +        end
> +
>         send(:function_include, vals)
>         vals = [vals] unless vals.is_a?(Array)
>
> -        # add a relation from ourselves to each required klass
>         vals.each do |klass|
> -            compiler.catalog.add_edge(resource, findresource 
> (:class, klass))
> +            # This is a bit hackish, in some ways, but it's the  
> only way
> +            # to configure a dependency that will make it to the  
> client.
> +            # The 'obvious' way is just to add an edge in the  
> catalog,
> +            # but that is considered a containment edge, not a  
> dependency
> +            # edge, so it usually gets lost on the client.
> +            ref = Puppet::Parser::Resource::Reference.new(:type  
> => :class, :title => klass)
> +            resource.set_parameter(:require, ref)
>         end
> end
> diff --git a/lib/puppet/parser/resource.rb b/lib/puppet/parser/ 
> resource.rb
> index 29b1fb1..b8aaf27 100644
> --- a/lib/puppet/parser/resource.rb
> +++ b/lib/puppet/parser/resource.rb
> @@ -175,6 +175,11 @@ class Puppet::Parser::Resource
>         end
>     end
>
> +    # Unless we're running >= 0.25, we're in compat mode.
> +    def metaparam_compatibility_mode?
> +        ! (catalog and version = catalog.client_version and version  
> = version.split(".") and version[0] == "0" and version[1].to_i >= 25)
> +    end
> +
>     # Return the resource name, or the title if no name
>     # was specified.
>     def name
> @@ -335,11 +340,6 @@ class Puppet::Parser::Resource
>         end
>     end
>
> -    # Unless we're running >= 0.25, we're in compat mode.
> -    def metaparam_compatibility_mode?
> -        ! (catalog and version = catalog.client_version and version  
> = version.split(".") and version[0] == "0" and version[1].to_i >= 25)
> -    end
> -
>     def add_scope_tags
>         if scope_resource = scope.resource
>             tag(*scope_resource.tags)
> diff --git a/spec/integration/parser/functions/require.rb b/spec/ 
> integration/parser/functions/require.rb
> index 79ba13d..960594b 100755
> --- a/spec/integration/parser/functions/require.rb
> +++ b/spec/integration/parser/functions/require.rb
> @@ -10,6 +10,7 @@ describe "the require function" do
>         @compiler = Puppet::Parser::Compiler.new(@node, @parser)
>
>         @compiler.send(:evaluate_main)
> +        @compiler.catalog.client_version = "0.25"
>         @scope = @compiler.topscope
>         # preload our functions
>         Puppet::Parser::Functions.function(:include)
> @@ -20,10 +21,11 @@ describe "the require function" do
>         @parser.newclass("requiredclass")
>
>         @scope.function_require("requiredclass")
> -
> -        @compiler.catalog.edge? 
> (@scope.resource,@compiler.findresource 
> (:class,"requiredclass")).should be_true
> +        @scope.resource["require"].should_not be_nil
> +        ref = @scope.resource["require"]
> +        ref.type.should == "Class"
> +        ref.title.should == "requiredclass"
>     end
> -
> end
>
> describe "the include function" do
> diff --git a/spec/unit/parser/functions/require.rb b/spec/unit/ 
> parser/functions/require.rb
> index 67a5bae..801c671 100755
> --- a/spec/unit/parser/functions/require.rb
> +++ b/spec/unit/parser/functions/require.rb
> @@ -7,8 +7,10 @@ describe "the require function" do
>     before :each do
>         @catalog = stub 'catalog'
>         @compiler = stub 'compiler', :catalog => @catalog
> +
> +        @resource = stub 'resource', :set_parameter =>  
> nil, :metaparam_compatibility_mode? => false
>         @scope = Puppet::Parser::Scope.new()
> -        @scope.stubs(:resource).returns("ourselves")
> +        @scope.stubs(:resource).returns @resource
>         @scope.stubs(:findresource)
>         @scope.stubs(:compiler).returns(@compiler)
>     end
> @@ -18,19 +20,19 @@ describe "the require function" do
>     end
>
>     it "should delegate to the 'include' puppet function" do
> -        @catalog.stubs(:add_edge)
>         @scope.expects(:function_include).with("myclass")
>
>         @scope.function_require("myclass")
>     end
>
> -    it "should add a catalog edge from our parent resource to the  
> included one" do
> -        @scope.stubs(:function_include).with("myclass")
> -        @scope.stubs(:findresource).with(:class, "myclass").returns 
> ("includedclass")
> -
> -        @catalog.expects(:add_edge).with("ourselves","includedclass")
> -
> +    it "should set the 'require' prarameter on the resource to a  
> resource reference" do
> +        @resource.expects(:set_parameter).with { |name, value| name  
> == :require and value.is_a?(Puppet::Parser::Resource::Reference) }
> +        @scope.stubs(:function_include)
>         @scope.function_require("myclass")
>     end
>
> +    it "should fail if used on a client not at least version 0.25" do
> +        @resource.expects(:metaparam_compatibility_mode? => true)
> +        lambda { @scope.function_require("myclass") }.should  
> raise_error(Puppet::ParseError)
> +    end
> end
> -- 
> 1.6.1
>
>
> >

--~--~---------~--~----~------------~-------~--~----~
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
-~----------~----~----~----~------~----~------~--~---

Reply via email to