On May 30, 2009, at 1:17 PM, Brice Figureau wrote:

>
> I don't know why we imposed the restriction that we shouldn't match
> with parameter containing arrays in exported mode.
> That doesn't seem right, as the produced rails query works fine with
> arrays.

I just noticed, too, that the current code doesn't allow conditionals  
in the Rails query, which also shouldn't be right.

I'm hesitant to fix it at this point, though, since we still need to  
move our Rails query to a more abstract, pluggable query (e.g.,  
something through a RESTful interface), so keeping it simple until  
that's done is a good idea.

>
> Note: the user tags are not stored in the rails database except under
> the special resource parameter tag. This also doesn't seem right.

Really?  So if someone sets 'tags' parameters on a resource, they're  
not stored in the db as parameters?

>
> Signed-off-by: Brice Figureau <brice-pup...@daysofwonder.com>
> ---
> lib/puppet/parser/ast/collexpr.rb |    2 +-
> spec/unit/parser/ast/collexpr.rb  |    8 +++++---
> 2 files changed, 6 insertions(+), 4 deletions(-)
>
> diff --git a/lib/puppet/parser/ast/collexpr.rb b/lib/puppet/parser/ 
> ast/collexpr.rb
> index 6ade58b..52f930e 100644
> --- a/lib/puppet/parser/ast/collexpr.rb
> +++ b/lib/puppet/parser/ast/collexpr.rb
> @@ -31,7 +31,7 @@ class CollExpr < AST::Branch
>             when "and"; code1.call(resource) and code2.call(resource)
>             when "or"; code1.call(resource) or code2.call(resource)
>             when "=="
> -                if resource[str1].is_a?(Array) && form != :exported
> +                if resource[str1].is_a?(Array)
>                     resource[str1].include?(str2)
>                 else
>                     resource[str1] == str2

I don't see how this code could fix things - this is only for the  
query against in-memory resources, not in the database, right?  So  
you'd match those resources from the current host, but not from all  
hosts.

I think there needs to be a special case for tags in the Rails query,  
just like there is for title.

And there should probably be something on the export side that unifies  
the tags set as parameters (e.g., tags => foo) and those set on the  
resource itself through parsing.

>
> diff --git a/spec/unit/parser/ast/collexpr.rb b/spec/unit/parser/ast/ 
> collexpr.rb
> index 5f0ca94..e2ad1f7 100755
> --- a/spec/unit/parser/ast/collexpr.rb
> +++ b/spec/unit/parser/ast/collexpr.rb
> @@ -73,17 +73,19 @@ describe Puppet::Parser::AST::CollExpr do
>             end
>         end
>     end
> -
> -    it "should check for array member equality if resource  
> parameter is an array for ==" do
> +
> +    [:exported,:virtual].each do |mode|
> +    it "should check for array member equality if resource  
> parameter is an array for == in mode #{mode}" do
>         array = mock 'array', :safeevaluate => "array"
>         test1 = mock 'test1'
>         test1.expects(:safeevaluate).with(@scope).returns("test1")
>
>         resource = mock 'resource'
>         resource.expects(: 
> []).with("array").at_least(1).returns(["test1","test2","test3"])
> -        collexpr = ast::CollExpr.new(:test1 => array, :test2 =>  
> test1, :oper => "==")
> +        collexpr = ast::CollExpr.new(:test1 => array, :test2 =>  
> test1, :oper => "==", :form => mode)
>         collexpr.evaluate(@scope)[1].call(resource).should be_true
>     end
> +    end
>
>     it "should raise an error for invalid operator" do
>         lambda { collexpr = ast::CollExpr.new(:oper=>">") }.should  
> raise_error
> -- 
> 1.6.0.2
>
>
> >


-- 
I was an only child... eventually. -- Stephen Wright
---------------------------------------------------------------------
Luke Kanies | http://reductivelabs.com | http://madstop.com


--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups 
"Puppet Developers" group.
To post to this group, send email to puppet-dev@googlegroups.com
To unsubscribe from this group, send email to 
puppet-dev+unsubscr...@googlegroups.com
For more options, visit this group at 
http://groups.google.com/group/puppet-dev?hl=en
-~----------~----~----~----~------~----~------~--~---

Reply via email to