Issue #17519 has been updated by Henrik Lindberg.

Category set to RAL
Status changed from Unreviewed to Accepted
Keywords changed from bd, language to bd, language, ral, type, provider

Types do not really have typed properties; they are strings, or enumerations. 
The designer(s) of this thought this would be enough, and that it was better to 
use an enumeration than a string (with pattern matching for "true", "false"). 
Currently that would be the alternative but using strings is not one bit better 
than the symbols.

The real solution is to allow types to have typed attributes (Numeric, Boolean, 
Enumeration, String, etc.) as well as validation of invariants (if x is set you 
must also set y; and variations on that theme). I am working on such a 
solution. At the same time, types should no longer be created as classes, but 
instead be instances to enable loading of multiple versions of types. (Multiple 
versions of a type makes environments blow up currently).

Given that the use of :true and :false is in quite a few places (I counted more 
than 20), it seems like there is plenty of opportunity to get burned fixing 
this. Making this change only for these boolean properties seems like 
duplication of effort given that a large portion of this has to be changed 
anyway. I have a suspicion that this may also have an effect on serialization 
and handling in the produced catalog. It also affects the API (or SPI if you 
prefer) for everyone writing types.

With that said, although it really works as intended, I am marking this as 
accepted, as this is an issue that a better type description/validation 
implementation should address. Let Booleans be Booleans...


----------------------------------------
Bug #17519: Puppet type and provider handling of boolean values.
https://projects.puppetlabs.com/issues/17519#change-76092

Author: Nan Liu
Status: Accepted
Priority: Normal
Assignee: Henrik Lindberg
Category: RAL
Target version: 
Affected Puppet version: 
Keywords: bd, language, ral, type, provider
Branch: 


So I have no idea why we convert boolean values to the symbol :true, :false, 
and this is making things a bit painful when writing puppet providers.

First, I'm not sure why this is necessary before we can even use true, false:

    newproperty(:example) do
      newvalues(:true, :false)
    end

Without this, the built in insync? always evaluate true. Once we added 
newvalues(:true, :false), we have to compare against symbol :true:

    /Stage[main]//Notify[trigger]/message: current_value absent, should be 
trigger (noop)
    /Stage[main]//Demo[ntpd]/running: current_value TrueClass true, should be 
Symbol :true (noop)
    Class[Main]: Would have triggered 'refresh' from 3 events
    Stage[main]: Would have triggered 'refresh' from 1 events
    Finished catalog run in 2.00 seconds

So far these work around doesn't work and they are both wat?:

* munge gets evaluated before whatever internal process changes the value to 
symbol :true, :false.
* insync? is not called if the value is :true, :false, so we can't change the 
comparison behavior.

You'll find these code examples in puppet provider comparing against the symbol 
true:

* @resource[:hasrestart] == :true
* if resource[:type_check] == :true

I'm curious why in the world we don't just treat them as booleans and instead 
as symbols and why we have do these special comparisons.


-- 
You have received this notification because you have either subscribed to it, 
or are involved in it.
To change your notification preferences, please click here: 
http://projects.puppetlabs.com/my/account

-- 
You received this message because you are subscribed to the Google Groups 
"Puppet Bugs" 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-bugs?hl=en.

Reply via email to