Do you rely on 'param=>undef' being equal to '(nothing)'?

[Eric Sorenson]

Hi, there's an issue that came up recently in the 3.0RCs -- Big thanks to Erik Dalén for reporting it in #16221 -- that involves a behaviour change to part of the DSL. In a nutshell, this code:

define foobar ($param='Hello world') {
notice($param)
}
foobar { 'test': param => undef }

in 2.7, causes 'Hello world' in the notice. In 3.x, it's nothing. As I said in the bug, this seems more correct to me -- I've overriden the default with an explicit 'undef', taking off the default. The same thing goes for invoking parameterised classes with undef arguments, which is perhaps more ambiguous (example from matthaus):

class toplevel (
$maybe = false,
$optional = undef ) {
if ($maybe) {
class { toplevel::secondlevel: optional => undef }
}
}

In order to make use of the default for the `optional` parameter in toplevel::secondlevel, you'd now need to either test in `toplevel` whether `$optional` was passed into it, or have toplevel::secondlevel use an `$optional_real` value inside it, similar to what's commonly done to append to defaults that are array values.

The closest thing to documentation around this suggests the new behaviour is what's intended <http://docs.puppetlabs.com/puppet/2.7/reference/lang_classes.html#overriding-resource-attributes>:

You can remove an attribute’s previous value without setting a new one by overriding it with the special value undef:

class base::freebsd inherits base::unix {
File['/etc/passwd'] {
group => undef,
}
}

So, I'm trying to determine whether this is a widespread pattern or an edge-case. Do you expect 'param=>undef' to be the same as not specifying param at all, or for the receiver to "see" the undef?

Eric Sorenson - eric.s...@puppetlabs.com
PuppetConf'12 - 27-28 Sep in SF - http://bit.ly/pcsig12

[Aaron Grewell]

I'm using the current behavior in inherited classes to unset parameters set by the parent class.  If that no longer works it will definitely impact my code.

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

[Stefan Schulte]

On Fri, Sep 14, 2012 at 11:31:08AM -0700, Eric Sorenson wrote:
> Hi, there's an issue that came up recently in the 3.0RCs -- Big thanks to Erik Dal�n for reporting it in #16221 -- that involves a behaviour change to part of the DSL. In a nutshell, this code:
>
[..]>

> class toplevel (
> $maybe = false,
> $optional = undef ) {
> if ($maybe) {
> class { toplevel::secondlevel: optional => undef }
> }
> }
>
> In order to make use of the default for the `optional` parameter in toplevel::secondlevel, you'd now need to either test in `toplevel` whether `$optional` was passed into it, or have toplevel::secondlevel use an `$optional_real` value inside it, similar to what's commonly done to append to defaults that are array values.
>

[...]

>
> So, I'm trying to determine whether this is a widespread pattern or an edge-case. Do you expect 'param=>undef' to be the same as not specifying param at all, or for the receiver to "see" the undef?
>
> Eric Sorenson - eric.s...@puppetlabs.com
> PuppetConf'12 - 27-28 Sep in SF - http://bit.ly/pcsig12
>

I use this a lot to be able to have an optional parameter in a parent
class that is passed to an included class and the included class
determines the default value. Like:

class basic($puppet_cron = undef) {
class { 'puppet::client':
cron => $puppet_cron,
}
}

-Stefan

[Trevor Vaughan]

I also use the undef to un-set parameters in various code segments. If
this is made real, it will impact my code.

Thanks,

Trevor

On Fri, Sep 14, 2012 at 6:39 PM, Stefan Schulte
<stefan....@taunusstein.net> wrote:
> On Fri, Sep 14, 2012 at 11:31:08AM -0700, Eric Sorenson wrote:

>> Hi, there's an issue that came up recently in the 3.0RCs -- Big thanks to Erik Dalén for reporting it in #16221 -- that involves a behaviour change to part of the DSL. In a nutshell, this code:

>>
> [..]>
>> class toplevel (
>> $maybe = false,
>> $optional = undef ) {
>> if ($maybe) {
>> class { toplevel::secondlevel: optional => undef }
>> }
>> }
>>
>> In order to make use of the default for the `optional` parameter in toplevel::secondlevel, you'd now need to either test in `toplevel` whether `$optional` was passed into it, or have toplevel::secondlevel use an `$optional_real` value inside it, similar to what's commonly done to append to defaults that are array values.
>>
> [...]
>>
>> So, I'm trying to determine whether this is a widespread pattern or an edge-case. Do you expect 'param=>undef' to be the same as not specifying param at all, or for the receiver to "see" the undef?
>>
>> Eric Sorenson - eric.s...@puppetlabs.com
>> PuppetConf'12 - 27-28 Sep in SF - http://bit.ly/pcsig12
>>
>
> I use this a lot to be able to have an optional parameter in a parent
> class that is passed to an included class and the included class
> determines the default value. Like:
>
> class basic($puppet_cron = undef) {
> class { 'puppet::client':
> cron => $puppet_cron,
> }
> }
>
> -Stefan
>

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

--
Trevor Vaughan
Vice President, Onyx Point, Inc
(410) 541-6699
tvau...@onyxpoint.com

-- This account not approved for unencrypted proprietary information --

[Henrik Lindberg]

On 2012-14-09 20:31, Eric Sorenson wrote:
> Hi, there's an issue that came up recently in the 3.0RCs -- Big thanks to Erik Dal�n for reporting it in #16221 -- that involves a behaviour change to part of the DSL. In a nutshell, this code:

>
> define foobar ($param='Hello world') {
> notice($param)
> }
> foobar { 'test': param => undef }
>
> in 2.7, causes 'Hello world' in the notice. In 3.x, it's nothing. As I said in the bug, this seems more correct to me -- I've overriden the default with an explicit 'undef', taking off the default. The same thing goes for invoking parameterised classes with undef arguments, which is perhaps more ambiguous (example from matthaus):
>
> class toplevel (
> $maybe = false,
> $optional = undef ) {
> if ($maybe) {
> class { toplevel::secondlevel: optional => undef }
> }
> }
>
> In order to make use of the default for the `optional` parameter in toplevel::secondlevel, you'd now need to either test in `toplevel` whether `$optional` was passed into it, or have toplevel::secondlevel use an `$optional_real` value inside it, similar to what's commonly done to append to defaults that are array values.
>
> The closest thing to documentation around this suggests the new behaviour is what's intended <http://docs.puppetlabs.com/puppet/2.7/reference/lang_classes.html#overriding-resource-attributes>:
>

> You can remove an attribute�s previous value without setting a new one by overriding it with the special value undef:

>
> class base::freebsd inherits base::unix {
> File['/etc/passwd'] {
> group => undef,
> }
> }
>
> So, I'm trying to determine whether this is a widespread pattern or an edge-case. Do you expect 'param=>undef' to be the same as not specifying param at all, or for the receiver to "see" the undef?
>
> Eric Sorenson - eric.s...@puppetlabs.com
> PuppetConf'12 - 27-28 Sep in SF - http://bit.ly/pcsig12
>

When we discussed this earlier, I also felt that the new behavior was
correct; settings something to undef erases the value, and if you want
the default, simply do not set the value.

However, seeing the examples, and thinking about it - if the change is
accepted it means it will be difficult to parameterize certain things -
what if there are many optional settings and some intermediary logic
takes a mix of parameters and needs to pass those on? Without the
ability to "end up with the default", but still enable passing a value,
there would need to be many different permutations selected with if
statements - and that would not be pretty. Alternatively, have the same
literal default values (which is not good as they would have to be kept
in sync).

Ideally, for this situation, using a literal 'default' would make the
code less confusing. I.e using the first example:

define foobar ($param='Hello world') {
notice($param)
}

foobar { 'test': param => default }

If this is implemented, it is possible to: set a new value, erase the
value, or use the default value if one is present.

... this is however a bigger thing to implement. Meanwhile, it may be
better to keep the somewhat mysterious behavior in 2.7.

Still feel that the behavior "undef erases the value" is a less
surprising result.

Regards
- henrik

[Eric Sorenson]

Aaron -- could you distill this down to a code sample? Unless I'm misunderstanding, it sounds like your case is slightly different to the ones I posted. Thanks.

[Daniel Pittman]

On Mon, Sep 17, 2012 at 11:58 AM, Eric Sorenson
<eric.s...@puppetlabs.com> wrote:
> Aaron -- could you distill this down to a code sample? Unless I'm
> misunderstanding, it sounds like your case is slightly different to the ones
> I posted. Thanks.

I recognise the case; this is a different use and, as far as I
understand, shouldn't be changed.

(It is also consistent with how the *new* behaviour of undef vs
parameter defaults works. :)

> "Puppet Users" group.
> To view this discussion on the web visit
> https://groups.google.com/d/msg/puppet-users/-/KtqHUAhcelcJ.
> To post to this group, send email to puppet...@googlegroups.com.

> To unsubscribe from this group, send email to

> puppet-users...@googlegroups.com.

> For more options, visit this group at

> http://groups.google.com/group/puppet-users?hl=en.



--
Daniel Pittman
⎋ Puppet Labs Developer – http://puppetlabs.com
♲ Made with 100 percent post-consumer electrons

[Aaron Grewell]

Well, we have a situation where an existing web server needs to be
managed but the application support folks have done something special.
So we have the "please don't break the existing config" class,
overriding our usual "you get the standard config so put your
specialness in httpd.conf.d" class.

class http::linux::custom inherits http::linux::install {
File [ "/etc/httpd/conf/httpd.conf" ]{
ensure => undef,
mode => undef,
source => undef,
owner => undef,
group => undef,
notify => undef,
require => undef,

[Erik Dalén]

That is not affected, only undef passed in parameters to classes and defines.

Erik Dalén

[Aaron Grewell]

Ah, OK. I'm not sure if I'm using default parameters in this way with
my defines. I'm not currently using parameterized classes so that's a
non-issue for me ATM.

I am concerned about the break in semantic consistency however. As
we've seen on the users list numerous times one of the challenges of
being a new Puppet user is the difficulty of tracking the delta
between what's acceptable in a class, a resource, and a define. This
seems like it would widen that semantic gap further.

[Daniel Pittman]

On Tue, Sep 18, 2012 at 2:25 PM, Aaron Grewell <aaron....@gmail.com> wrote:

> Ah, OK. I'm not sure if I'm using default parameters in this way with
> my defines. I'm not currently using parameterized classes so that's a
> non-issue for me ATM.
>
> I am concerned about the break in semantic consistency however. As
> we've seen on the users list numerous times one of the challenges of
> being a new Puppet user is the difficulty of tracking the delta
> between what's acceptable in a class, a resource, and a define. This
> seems like it would widen that semantic gap further.

Doesn't it close that gap up?

Now, instead of passing 'undef' to a define meaning "use the default
in the code", it means "use undef" - just like it does when you set
defaults as you described.

[Aaron Grewell]

On Tue, Sep 18, 2012 at 2:44 PM, Daniel Pittman <dan...@puppetlabs.com> wrote:
> On Tue, Sep 18, 2012 at 2:25 PM, Aaron Grewell <aaron....@gmail.com> wrote:

> Doesn't it close that gap up?
>
> Now, instead of passing 'undef' to a define meaning "use the default
> in the code", it means "use undef" - just like it does when you set
> defaults as you described.
>

Let me see if I understand. Given this example:

__________________
service { httpd: ensure => undef }

define webserver ( $ensure=stopped ) {
service { httpd: ensure => $ensure }
}

webserver { ws_foo: ensure => undef }
__________________

The old behavior would mean that the bare service resource would not
affect httpd's status, whereas the define would stop the service by
using the default value. The new behavior would mean that the two
would behave identically. Does that sound right?

I'll have to look through the code to see if I'm depending on this
anywhere, but I don't think so. I agree that the new behavior is more
consistent. I can see some unhappiness from users expecting to use
this to avoid input handling though.

[Daniel Pittman]

On Tue, Sep 18, 2012 at 3:09 PM, Aaron Grewell <aaron....@gmail.com> wrote:
> On Tue, Sep 18, 2012 at 2:44 PM, Daniel Pittman <dan...@puppetlabs.com> wrote:
>> On Tue, Sep 18, 2012 at 2:25 PM, Aaron Grewell <aaron....@gmail.com> wrote:
>
>> Doesn't it close that gap up?
>>
>> Now, instead of passing 'undef' to a define meaning "use the default
>> in the code", it means "use undef" - just like it does when you set
>> defaults as you described.
>>
>
> Let me see if I understand. Given this example:
>
> __________________
> service { httpd: ensure => undef }
>
> define webserver ( $ensure=stopped ) {
> service { httpd: ensure => $ensure }
> }
>
> webserver { ws_foo: ensure => undef }
> __________________
>
> The old behavior would mean that the bare service resource would not
> affect httpd's status, whereas the define would stop the service by
> using the default value. The new behavior would mean that the two
> would behave identically. Does that sound right?

Yes, that is correct.

> I'll have to look through the code to see if I'm depending on this
> anywhere, but I don't think so. I agree that the new behavior is more
> consistent. I can see some unhappiness from users expecting to use
> this to avoid input handling though.

Yeah - I (and I know Eric) are worried about the potential impact,
but, as you say, it brings things to a more consistent point. To me
that seems worthwhile.

[Eric Sorenson]

On Friday, September 14, 2012 3:36:20 PM UTC-7, Stefan Schulte wrote:

I use this a lot to be able to have an optional parameter in a parent
class that is passed to an included class and the included class
determines the default value. Like:

    class basic($puppet_cron = undef) {
      class { 'puppet::client':
        cron => $puppet_cron,
      }
    }

Stefan - This is exactly the case I'm concerned about.

The simplest way to preserve this pattern would be putting the default value for cron in the `basic` typedef, where you now have `undef`.

class basic($puppet_cron = "some_default") {

   class { 'puppet::client': 

      cron => $puppet_cron,

  }

}

The advantage is that if you want to really revert to the default for the `cron` parameter, you can actually invoke it with undef, just like a regular resource.

The bad side is that you now have to move your defaults to the calling class, or worse, duplicate them.

What do you think?

-=Eric

[Eric Sorenson]

On Saturday, September 15, 2012 5:06:52 PM UTC-7, Trevor Vaughan wrote:

I also use the undef to un-set parameters in various code segments. If
this is made real, it will impact my code.

Trevor, if I understand correctly I think using undef to un-set parameters won't change. If anything it will be preserved in cases (i.e. into defined types) where it currently gets silently dropped. Do you have a little code sample?

-=Eric

[Stefan Schulte]

Eric -

In my case a class that is included in a wrapper class is not supposed
to be included directly at node level. So I could move the default
values in the wrapper class.

This would also make documenting the wrapper class cleaner (I dont have
to write "param foo determines bar. For the default value check the
documentation of class X"). On the other hand I am not able to include
the class directly anymore without specifying all paramters explicitly
(as I said I currently don't do that, but others might). Duplicating the
default value seems like a bad thing to do, especially when they get out
of sync. And I am not sure if I like the fact that the knowledge "what
is a sane default value for this particular parameter" is shifting out
of the actual class.

-Stefan

[Erik Dalén]

Another possibility is to use the pattern in puppetlabs-mysql where
mysql::server wraps the inner class mysql::config.
See https://github.com/puppetlabs/puppetlabs-mysql/blob/master/manifests/server.pp
lines 28-30.

Basically you pass the parameters as a hash to the outer class which
it then uses with the create_resources() function to create a inner
class.

Then you can use the parameter defaults feature of the
create_resources function to pass any extra parameters to the inner
class, or use the merge() function from stdlib to merge two hashes
together to pass some extra parameters.

Switching to this pattern might involve some refactoring though for people.

--
Erik Dalén

[Trevor Vaughan]

After reading through the follow up messages, I think that the
proposed change won't affect me at all.

Thanks for the follow up.

Trevor

> --
> You received this message because you are subscribed to the Google Groups
> "Puppet Developers" group.

> To view this discussion on the web visit

> https://groups.google.com/d/msg/puppet-dev/-/LzxOfpLRUTUJ.

[Eric Sorenson]

Friends -- 

As of 30da528e9c8ac345d1020d1cbeee02598174a3c5 we put this intentionally back to the 2.7 behaviour. Because both

(a) the change crept in as a side-effect of other code changes, not as a deliberate language design choice

 -and-

(b) we are not providing a simple (sigil or keyword) workaround for people to preserve the behaviour they've written around.

are true here, I'm convinced it's not a good change. After this commit, the sample code in Erik's update 1 works as expected, as do the other cases I'm able to test.

Thanks for your feedback.

Eric Sorenson - eric.s...@puppetlabs.com 
PuppetConf'12 - 27-28 Sep in SF - http://bit.ly/pcsig12