On 31/05/09 1:15, Luke Kanies wrote:
> 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.
It's simple. It's just matching multi-valued parameters.
I think I didn't explain correctly what the problem is.
The in-memory query forbids matchin multi-valued parameters, but the
rails one allows it by design.
Ex:
@@file {
..., tag => ["a","b"]
}
File<<| tag == "a" |>>
This won't match for the host who created the resource when it collects
it, but will work for other hosts, because the rails query is (pseudo sql):
param_name.value="tag" and param_value.value="a"
which works fine for multi-valued parameters.
>> 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?
No they're stored as a parameter but not as a tag.
And the built rails query matches tag as parameter.
So you can't do:
class myfile {
@@file { ... }
}
File<<| tag == "myfile" |>>
My idea would be that we should persist user tag as tag _and_ parameter
and that tag queries should query the persisted tags.
>> 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.
Correct. The other host case was already covered by the rails query
produced.
> I think there needs to be a special case for tags in the Rails query,
> just like there is for title.
Yes.
> 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.
Correct, too, hence my side not in my original commit msg.
So what's the plan?
* leave it as is?
or
* apply this patch?
or
* enhance the patch to unify tags (coming for the tag parameter, the
tag function, the scope) in tag db and adapt the rails query?
Choose your poison :-)
--
Brice Figureau
My Blog: http://www.masterzen.fr/
--~--~---------~--~----~------------~-------~--~----~
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
-~----------~----~----~----~------~----~------~--~---
