Puppet users - Jan 2012 - RFC: Deprecate defined() function for Telly.

I''m looking for strong opinions on whether we should or
shouldn''t deprecate
the defined() function for Telly, the next major Puppet release this year.

jcbollinger put it quite well in another thread:

"Use of the "defined" function introduces a parse-order
dependency, and the
additional work you need to do to ensure that that dependency is always
fulfilled overcomes any simplicity advantage that might otherwise exist."



-- 
Nigel Kersten
Product Manager, Puppet Labs

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

----- Original Message -----
> I''m looking for strong opinions on whether we should or
shouldn''t
> deprecate the defined() function for Telly, the next major Puppet
> release this year.
First choice would be to make it reliable, that seems unlikely though

+1, I''d also make the scope.classes variable private somehow for
similar
reasons.

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

+1 to ditch it. As the other thread said well: use of defined() is
only done in badly designed code. Getting rid of it will force people
to use better designs.

cheers,

Walter Heck

On Thu, Jan 19, 2012 at 19:19, R.I.Pienaar <rip@devco.net>
wrote:
>
>
> ----- Original Message -----
>> I''m looking for strong opinions on whether we should or
shouldn''t
>> deprecate the defined() function for Telly, the next major Puppet
>> release this year.
>
> First choice would be to make it reliable, that seems unlikely though
>
> +1, I''d also make the scope.classes variable private somehow for
similar
> reasons.
>
> --
> You received this message because you are subscribed to the Google Groups
"Puppet Users" group.
> To post to this group, send email to puppet-users@googlegroups.com.
> To unsubscribe from this group, send email to
puppet-users+unsubscribe@googlegroups.com.
> For more options, visit this group at
http://groups.google.com/group/puppet-users?hl=en.
>


-- 
Walter Heck

--
follow @walterheck on twitter to see what I''m up to!
--
Check out my new startup: Server Monitoring as a Service @ http://tribily.com
Follow @tribily on Twitter and/or ''Like'' our Facebook page at
http://www.facebook.com/tribily

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

Defined() doesn''t suck! It''s a 100% reliable way to check what
classes
and defined types are available to the autoloader. I challenge anyone
to find me an example of this usage that fails.

But trying to use it to see whether a specific resource instance has
been declared is pure madness and must be stopped. It''s defined(), not
declared(), and it shouldn''t accept resource references as arguments
at all.

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

----- Original Message -----
> Defined() doesn''t suck! It''s a 100% reliable way to check
what
> classes and defined types are available to the autoloader. I challenge
anyone
> to find me an example of this usage that fails.
can you give an example of this use case pls?

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

On Jan 19, 11:01 am, "R.I.Pienaar" <r...@devco.net>
wrote:
> ----- Original Message -----
> > Defined() doesn''t suck! It''s a 100% reliable way to
check what
> > classes and defined types are available to the autoloader. I challenge
anyone
> > to find me an example of this usage that fails.
>
> can you give an example of this use case pls?
Well... that''s something I realized after I posted that, is
I''m not
sure if anyone WANTS a reliable way to test the autoloader. (Obviously
people do want a way to check for resource instances, which is why
defined() keeps getting used for that...)

But anyway! Say you make a module for a network service and you want
it to be able to manage its own firewall rule. You know of a defined
type for firewall rules, and you''re using it, but you want your module
to be portable and you know of good reasons why someone wouldn''t be
using your iptables module.

So, you can conditionally declare the rule if the defined type is
available to the autoloader, and otherwise you don''t attempt to manage
the firewall and expect that the user has read the documentation and
will make a hole for the service themselves.

if defined(firewall::iptables::rule) {
  firewall::iptables::rule {''mysql_server'':
    ...etc. etc.
  }
}

See? It''s just a way to peek around at what the user has installed.

Which... maybe implies that it should be renamed to "installed()."
Dunno.

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

----- Original Message -----
> 
> 
> On Jan 19, 11:01 am, "R.I.Pienaar" <r...@devco.net> wrote:
> > ----- Original Message -----
> > > Defined() doesn''t suck! It''s a 100% reliable
way to check what
> > > classes and defined types are available to the autoloader. I
> > > challenge anyone
> > > to find me an example of this usage that fails.
> >
> > can you give an example of this use case pls?
> 
> Well... that''s something I realized after I posted that, is
I''m not
> sure if anyone WANTS a reliable way to test the autoloader.
> (Obviously
> people do want a way to check for resource instances, which is why
> defined() keeps getting used for that...)
> 
> But anyway! Say you make a module for a network service and you want
> it to be able to manage its own firewall rule. You know of a defined
> type for firewall rules, and you''re using it, but you want your
> module
> to be portable and you know of good reasons why someone wouldn''t
be
> using your iptables module.
> 
> So, you can conditionally declare the rule if the defined type is
> available to the autoloader, and otherwise you don''t attempt to
> manage
> the firewall and expect that the user has read the documentation and
> will make a hole for the service themselves.
> 
> if defined(firewall::iptables::rule) {
>   firewall::iptables::rule {''mysql_server'':
>     ...etc. etc.
>   }
> }
> 
> See? It''s just a way to peek around at what the user has
installed.
right, I must have missed this behavior previously, yeah this has value
and should be retained, checking for declared resources should go then

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

I use defined every so often so I would be sad if it was removed, it''s
handy for certain things of things:

common/foreman_proxy/manifests/params.pp:  if
defined(Class[''puppet::server::ca'']) {
common/foreman_proxy/manifests/params.pp:  } elsif
defined(Class[''puppet::server'']) {
common/foreman_proxy/manifests/params.pp:  if
defined(Class[''foreman_proxy::role::build'']) {
common/mysql/manifests/rights.pp:    if ! defined(Mysql_user
["${user}@${host}"])
{

An example:

    if ! defined(Mysql_user ["${user}@${host}"]) {
      mysql_user { "${user}@${host}":
        password_hash => mysql_password($password),
        require => File["/root/.my.cnf"],
      }
    }

In my foreman_proxy stuff:

  # puppetca settings
  if defined(Class[''puppet::server::ca'']) {
    $puppetca          = true
    $autosign_location = "/etc/puppet/autosign.conf"
    $puppetca_cmd      = "/usr/sbin/puppetca"
    $puppet_group      = "puppet"

    # puppetrun settings
    $puppetrun     = true
    $puppetrun_cmd = "/usr/sbin/puppetrun"
  } elsif defined(Class[''puppet::server'']) {
    # puppetrun settings
    $puppetca      = false
    $puppetrun     = true
    $puppetrun_cmd = "/usr/sbin/puppetrun"
  } else {
    $puppetca = false
    $puppetrun = false
  }

I''m sure there are other ways to do it but this seems elegant and makes
sense at times.  I could set variables in those other classes and then
check for them I suppose but what is the harm in doing things this way?  I
don''t see why defined() couldn''t be implemented properly
rather than being
removed.

On Thu, Jan 19, 2012 at 3:17 PM, Nick Fagerlund <
nick.fagerlund@puppetlabs.com> wrote:
>
>
> On Jan 19, 11:01 am, "R.I.Pienaar" <r...@devco.net> wrote:
> > ----- Original Message -----
> > > Defined() doesn''t suck! It''s a 100% reliable
way to check what
> > > classes and defined types are available to the autoloader. I
challenge
> anyone
> > > to find me an example of this usage that fails.
> >
> > can you give an example of this use case pls?
>
> Well... that''s something I realized after I posted that, is
I''m not
> sure if anyone WANTS a reliable way to test the autoloader. (Obviously
> people do want a way to check for resource instances, which is why
> defined() keeps getting used for that...)
>
> But anyway! Say you make a module for a network service and you want
> it to be able to manage its own firewall rule. You know of a defined
> type for firewall rules, and you''re using it, but you want your
module
> to be portable and you know of good reasons why someone wouldn''t
be
> using your iptables module.
>
> So, you can conditionally declare the rule if the defined type is
> available to the autoloader, and otherwise you don''t attempt to
manage
> the firewall and expect that the user has read the documentation and
> will make a hole for the service themselves.
>
> if defined(firewall::iptables::rule) {
>  firewall::iptables::rule {''mysql_server'':
>    ...etc. etc.
>  }
> }
>
> See? It''s just a way to peek around at what the user has
installed.
>
> Which... maybe implies that it should be renamed to
"installed()."
> Dunno.
>
> --
> You received this message because you are subscribed to the Google Groups
> "Puppet Users" group.
> To post to this group, send email to puppet-users@googlegroups.com.
> To unsubscribe from this group, send email to
> puppet-users+unsubscribe@googlegroups.com.
> For more options, visit this group at
> http://groups.google.com/group/puppet-users?hl=en.
>
>
-- 
You received this message because you are subscribed to the Google Groups
"Puppet Users" group.
To post to this group, send email to puppet-users@googlegroups.com.
To unsubscribe from this group, send email to
puppet-users+unsubscribe@googlegroups.com.
For more options, visit this group at
http://groups.google.com/group/puppet-users?hl=en.

But this is begging for trouble:

On 01/19/2012 09:22 PM, Ashley Penney wrote:
> An example:
> 
>     if ! defined(Mysql_user ["${user}@${host}"]) {
>       mysql_user { "${user}@${host}":
>         password_hash => mysql_password($password),
>         require => File["/root/.my.cnf"],
>       }
>     }
If your master processes this before the *other* declaration of that
mysql_user{}, you''re back to square one and get errors about multiple
resource declaration.

I can see only pain down this road.

Nick''s example on the other hand is quite enticing, I think. We want to
keep that (and include it in some Best Practices ;-)

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

I wind up using defined more than I should probably admit. yes it is
dangerous/confusing b/c of parse order dependencies, but it is also really
useful for a few use cases

* static resources in a defined resource type (avoids having to use classes
to store all static dependencies)

* the big reason I keep on leaning on it is for package dependencies. Often
something needs an additional package installed (and it is possible that
other modules may have that same package dependency, and I don''t want
to
have to create a new class every time that I need another package
(especially for something complicated that may have tons of package
dependencies)

puppet-apt has a relevant pull request where someone wants to wrap a
defined? around python-software-properties for this exact reason

https://github.com/puppetlabs/puppet-apt/pull/10

On Thu, Jan 19, 2012 at 9:18 AM, Nigel Kersten <nigel@puppetlabs.com>
wrote:
> I''m looking for strong opinions on whether we should or
shouldn''t
> deprecate the defined() function for Telly, the next major Puppet release
> this year.
>
> jcbollinger put it quite well in another thread:
>
> "Use of the "defined" function introduces a parse-order
dependency, and
> the additional work you need to do to ensure that that dependency is always
> fulfilled overcomes any simplicity advantage that might otherwise
exist."
>
>
>
> --
> Nigel Kersten
> Product Manager, Puppet Labs
>
>
>  --
> You received this message because you are subscribed to the Google Groups
> "Puppet Users" group.
> To post to this group, send email to puppet-users@googlegroups.com.
> To unsubscribe from this group, send email to
> puppet-users+unsubscribe@googlegroups.com.
> For more options, visit this group at
> http://groups.google.com/group/puppet-users?hl=en.
>
-- 
You received this message because you are subscribed to the Google Groups
"Puppet Users" group.
To post to this group, send email to puppet-users@googlegroups.com.
To unsubscribe from this group, send email to
puppet-users+unsubscribe@googlegroups.com.
For more options, visit this group at
http://groups.google.com/group/puppet-users?hl=en.

How about deprecating defined?(Type[''title'']), but allowing it
to accept a
resource hash? This would definitely satisfy my use cases while alleviating
concerns about resource attribute conflicts/parse order dependencies

if defined?(
 {
    package[''foo''] => { ensure => present }
  }
) {
  package { ''foo'': ensure => present }
}

On Fri, Jan 20, 2012 at 1:00 AM, Dan Bode <dan@puppetlabs.com> wrote:
> I wind up using defined more than I should probably admit. yes it is
> dangerous/confusing b/c of parse order dependencies, but it is also really
> useful for a few use cases
>
> * static resources in a defined resource type (avoids having to use
> classes to store all static dependencies)
>
> * the big reason I keep on leaning on it is for package dependencies.
> Often something needs an additional package installed (and it is possible
> that other modules may have that same package dependency, and I
don''t want
> to have to create a new class every time that I need another package
> (especially for something complicated that may have tons of package
> dependencies)
>
> puppet-apt has a relevant pull request where someone wants to wrap a
> defined? around python-software-properties for this exact reason
>
> https://github.com/puppetlabs/puppet-apt/pull/10
>
>
> On Thu, Jan 19, 2012 at 9:18 AM, Nigel Kersten
<nigel@puppetlabs.com>wrote:
>
>> I''m looking for strong opinions on whether we should or
shouldn''t
>> deprecate the defined() function for Telly, the next major Puppet
release
>> this year.
>>
>> jcbollinger put it quite well in another thread:
>>
>> "Use of the "defined" function introduces a parse-order
dependency, and
>> the additional work you need to do to ensure that that dependency is
always
>> fulfilled overcomes any simplicity advantage that might otherwise
exist."
>>
>>
>>
>> --
>> Nigel Kersten
>> Product Manager, Puppet Labs
>>
>>
>>  --
>> You received this message because you are subscribed to the Google
Groups
>> "Puppet Users" group.
>> To post to this group, send email to puppet-users@googlegroups.com.
>> To unsubscribe from this group, send email to
>> puppet-users+unsubscribe@googlegroups.com.
>> For more options, visit this group at
>> http://groups.google.com/group/puppet-users?hl=en.
>>
>
>
-- 
You received this message because you are subscribed to the Google Groups
"Puppet Users" group.
To post to this group, send email to puppet-users@googlegroups.com.
To unsubscribe from this group, send email to
puppet-users+unsubscribe@googlegroups.com.
For more options, visit this group at
http://groups.google.com/group/puppet-users?hl=en.

Hi Dan,

sorry if I come around bluntly about this, but:

On 01/20/2012 10:00 AM, Dan Bode wrote:
> * static resources in a defined resource type (avoids having to use
> classes to store all static dependencies)
> 
> * the big reason I keep on leaning on it is for package dependencies.
> Often something needs an additional package installed (and it is
> possible that other modules may have that same package dependency, and I
> don''t want to have to create a new class every time that I need
another
> package (especially for something complicated that may have tons of
> package dependencies)
I disagree. The coding overhead for a simple wrapper class is not much
larger than adding if defined(); of course, there is the matter of
organizing those wrappers in your modules, though.
Still, endorsing defined() abuse in such a manner will lead to bad (and
ugly) code all over the place (how do you protect against redeclaration
outside your own defined type?)
> puppet-apt has a relevant pull request where someone wants to wrap a
> defined? around python-software-properties for this exact reason
> 
> https://github.com/puppetlabs/puppet-apt/pull/10
Again, I find the endorsement of clumsy design questionable. Common
dependencies should be isolated and exported into a mutual dependency
(module).

If there are real life difficulties that make such workflows impossible
at the moment, I still think that it''s a bad idea to root flaw-prone
workarounds ever deeper into the module pool.
> How about deprecating defined?(Type[''title'']), but
allowing it to accept
> a resource hash? This would definitely satisfy my use cases while
> alleviating concerns about resource attribute conflicts/parse order
> dependencies
>
> if defined?(
>  {
>     package[''foo''] => { ensure => present }
>   }
> ) {
>   package { ''foo'': ensure => present }
> }
Interesting. This will add some complexity to the DSL, which I''d be
wary
of, but if there are truly compelling use cases, this might be
worthwile? I''m not sure. Does this not suffer from the same problems
the
current defined() has?

Sincerely,
Felix

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

On Thu, Jan 19, 2012 at 12:17 PM, Nick Fagerlund
<nick.fagerlund@puppetlabs.com> wrote:
>
>
> On Jan 19, 11:01 am, "R.I.Pienaar" <r...@devco.net> wrote:
>> ----- Original Message -----
>> > Defined() doesn''t suck! It''s a 100% reliable way
to check what
>> > classes and defined types are available to the autoloader. I
challenge anyone
>> > to find me an example of this usage that fails.
>>
>> can you give an example of this use case pls?
>
> Well... that''s something I realized after I posted that, is
I''m not
> sure if anyone WANTS a reliable way to test the autoloader. (Obviously
> people do want a way to check for resource instances, which is why
> defined() keeps getting used for that...)
>
> But anyway! Say you make a module for a network service and you want
> it to be able to manage its own firewall rule. You know of a defined
> type for firewall rules, and you''re using it, but you want your
module
> to be portable and you know of good reasons why someone wouldn''t
be
> using your iptables module.
>
> So, you can conditionally declare the rule if the defined type is
> available to the autoloader, and otherwise you don''t attempt to
manage
> the firewall and expect that the user has read the documentation and
> will make a hole for the service themselves.
>
> if defined(firewall::iptables::rule) {
>  firewall::iptables::rule {''mysql_server'':
>    ...etc. etc.
>  }
> }
>
> See? It''s just a way to peek around at what the user has
installed.
>
> Which... maybe implies that it should be renamed to
"installed()."
> Dunno.
So there is a patch for your specific use case. We should at least
differentiate defined vs. declared and a patch was written to split
off defined() vs. declared():

http://projects.puppetlabs.com/issues/3124

The warning is simply:

Puppet.warning "Detecting puppet resource via defined() function is
deprecated, use declared(). Detecting whether a class or resource type
is defined is still supported."

I think this part of the functionality makes sense to retain. For
those who care, feel free to take a peak at the code:
https://github.com/nanliu/puppet/tree/ticket/2.7.x/3124

Thanks,

Nan

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

What would you recommend as an alternative way to handle these cases?  I
suppose the mysql lib could be extended to be able to check for users (not
easily, but it could be done), but what about in the second case where I
want to check for various roles being set as classes and then use those to
decide the configuration of foreman.  Volcane said that setting variables
in the other classes and checking for those isn''t going to cut it
either.
 What''s a good pattern for this?

On Fri, Jan 20, 2012 at 3:39 AM, Felix Frank <
felix.frank@alumni.tu-berlin.de> wrote:
> But this is begging for trouble:
>
> On 01/19/2012 09:22 PM, Ashley Penney wrote:
> > An example:
> >
> >     if ! defined(Mysql_user ["${user}@${host}"]) {
> >       mysql_user { "${user}@${host}":
> >         password_hash => mysql_password($password),
> >         require => File["/root/.my.cnf"],
> >       }
> >     }
>
> If your master processes this before the *other* declaration of that
> mysql_user{}, you''re back to square one and get errors about
multiple
> resource declaration.
>
> I can see only pain down this road.
>
> Nick''s example on the other hand is quite enticing, I think. We
want to
> keep that (and include it in some Best Practices ;-)
>
> --
> You received this message because you are subscribed to the Google Groups
> "Puppet Users" group.
> To post to this group, send email to puppet-users@googlegroups.com.
> To unsubscribe from this group, send email to
> puppet-users+unsubscribe@googlegroups.com.
> For more options, visit this group at
> http://groups.google.com/group/puppet-users?hl=en.
>
>
-- 
You received this message because you are subscribed to the Google Groups
"Puppet Users" group.
To post to this group, send email to puppet-users@googlegroups.com.
To unsubscribe from this group, send email to
puppet-users+unsubscribe@googlegroups.com.
For more options, visit this group at
http://groups.google.com/group/puppet-users?hl=en.

On Jan 19, 2:17 pm, Nick Fagerlund <nick.fagerl...@puppetlabs.com>
wrote:
> On Jan 19, 11:01 am, "R.I.Pienaar" <r...@devco.net> wrote:
>
> > ----- Original Message -----
> > > Defined() doesn''t suck! It''s a 100% reliable
way to check what
> > > classes and defined types are available to the autoloader. I
challenge anyone
> > > to find me an example of this usage that fails.
>
> > can you give an example of this use case pls?
>
> Well... that''s something I realized after I posted that, is
I''m not
> sure if anyone WANTS a reliable way to test the autoloader. (Obviously
> people do want a way to check for resource instances, which is why
> defined() keeps getting used for that...)
>
> But anyway! Say you make a module for a network service and you want
> it to be able to manage its own firewall rule. You know of a defined
> type for firewall rules, and you''re using it, but you want your
module
> to be portable and you know of good reasons why someone wouldn''t
be
> using your iptables module.
>
> So, you can conditionally declare the rule if the defined type is
> available to the autoloader, and otherwise you don''t attempt to
manage
> the firewall and expect that the user has read the documentation and
> will make a hole for the service themselves.
>
> if defined(firewall::iptables::rule) {
>   firewall::iptables::rule {''mysql_server'':
>     ...etc. etc.
>   }
>
> }
>
> See? It''s just a way to peek around at what the user has
installed.
>
> Which... maybe implies that it should be renamed to
"installed()."
> Dunno.

I don''t think I would ever write code like that myself.  I would
rather document module and class dependencies, and expect to have
catalog compilation fail if I try to use classes or other module
features whose documented dependencies are not available.  That''s a
style and best practices position, however.  I would be satisfied to
have only defined()''s support for resource references be deprecated.

Alternatively, defined() could be wholly deprecated, but a new
function (e.g. "installed()") introduced that is a work-alike except
for not accepting resource references.


John

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

On Jan 20, 3:00 am, Dan Bode <d...@puppetlabs.com>
wrote:
> * the big reason I keep on leaning on it is for package dependencies. Often
> something needs an additional package installed (and it is possible that
> other modules may have that same package dependency, and I don''t
want to
> have to create a new class every time that I need another package
> (especially for something complicated that may have tons of package
> dependencies)

You describe one of the core use cases for virtual resources.  Instead
of relying on the defined() function, you can
1) define virtual Package resources in some central place(s) for all
the packages your nodes might want to manage,
2) include that class wherever needed, and
3) *realize* Packages as appropriate wherever you know you need
certain ones.

That avoids parse-order issues, doesn''t require you to keep multiple
definitions of the same resource synchronized, and makes it easier to
find specific Package definitions among your manifests.  In some cases
you might even be able to leverage collections with selection
predicates to simplify and clarify your manifests.  I don''t see a
single reason to prefer use of ''defined'' for this case.


John

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

On Jan 20, 3:38 am, Nan Liu <n...@puppetlabs.com>
wrote:
> So there is a patch for your specific use case. We should at least
> differentiate defined vs. declared and a patch was written to split
> off defined() vs. declared():
>
> http://projects.puppetlabs.com/issues/3124
>
> The warning is simply:
>
> Puppet.warning "Detecting puppet resource via defined() function is
> deprecated, use declared(). Detecting whether a class or resource type
> is defined is still supported."
>
> I think this part of the functionality makes sense to retain. For
> those who care, feel free to take a peak at the
code:https://github.com/nanliu/puppet/tree/ticket/2.7.x/3124

So perhaps declared() should be deprecated, with the suggestion to use
it being removed from the warning message emitted by defined().


John

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

On Jan 20, 6:39 am, jcbollinger <John.Bollin...@stJude.org>
wrote:
> On Jan 20, 3:00 am, Dan Bode <d...@puppetlabs.com> wrote:
>
> > * the big reason I keep on leaning on it is for package dependencies.
Often
> > something needs an additional package installed (and it is possible
that
> > other modules may have that same package dependency, and I
don''t want to
> > have to create a new class every time that I need another package
> > (especially for something complicated that may have tons of package
> > dependencies)
>
> You describe one of the core use cases for virtual resources.  Instead
> of relying on the defined() function, you can
> 1) define virtual Package resources in some central place(s) for all
> the packages your nodes might want to manage,
> 2) include that class wherever needed, and
> 3) *realize* Packages as appropriate wherever you know you need
> certain ones.
>
> That avoids parse-order issues, doesn''t require you to keep
multiple
> definitions of the same resource synchronized, and makes it easier to
> find specific Package definitions among your manifests.  In some cases
> you might even be able to leverage collections with selection
> predicates to simplify and clarify your manifests.  I don''t see a
> single reason to prefer use of ''defined'' for this case.
Defining all somewhat common packages in a central location becomes
unrealistic when you no longer "control" the code that is in every
module you use.  If you obtain five modules from the forge and they
all require a specific package and so all define that package your not
going to convince, nor is it a good design to require everyone to move
the package definitions from that collection of modules.  They need to
function as a collection out of the box.

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

On Fri, Jan 20, 2012 at 2:34 PM, Cody <c.a.herriges@gmail.com>
wrote:
> Defining all somewhat common packages in a central location becomes
> unrealistic when you no longer "control" the code that is in
every
> module you use.  If you obtain five modules from the forge and they
> all require a specific package and so all define that package your not
> going to convince, nor is it a good design to require everyone to move
> the package definitions from that collection of modules.  They need to
> function as a collection out of the box.
>
Are we sure it can''t be fixed?  What makes defined() so different from
the code that implements require?  Shouldn''t "if  not
defined" be the
same as "if a require would fail"?  That seems to be what people are
expecting, why not give it to them?

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

On Fri, Jan 20, 2012 at 2:49 PM, Aaron Grewell <aaron.grewell@gmail.com>
wrote:
>
> Are we sure it can''t be fixed?  What makes defined() so different
from
> the code that implements require?  Shouldn''t "if  not
defined" be the
> same as "if a require would fail"?  That seems to be what people
are
> expecting, why not give it to them?
Never mind that last bit, it took me a second to realize the order of
operations would make that Very Hard.  A better question would be "do
we have a proper replacement in the pipeline for Telly?"  Perhaps
exception handling for require?  If not then something''s better than
nothing IMHO.

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

Hi,

On 01/20/2012 11:34 PM, Cody wrote:
> Defining all somewhat common packages in a central location becomes
> unrealistic when you no longer "control" the code that is in
every
> module you use.  If you obtain five modules from the forge and they
> all require a specific package and so all define that package your not
> going to convince, nor is it a good design to require everyone to move
> the package definitions from that collection of modules.  They need to
> function as a collection out of the box.
Agreed. How can this be accomplished?

Perhaps there needs to be some kind of "Forge common" module that by
policy can only ever declare virtual resources (packages are a prominent
example).
A user who wishes to retain the capability of using modules from the
Forge would be required to install this common module, and replace their
own resource declarations with realizations of the common resources.
For this to work, it''s definitely a plus that you can override
attributes in collections:
Package<| title == "apache2": |> { ensure =>
"2.2.12" }
...although that does bear some caveats. Does this still work in recent
versions?

If we can take this for granted, all Forge modules can adhere to that
same standard.

Yes, it''s quite a hassle.

No, I didn''t think this through very thoroughly ;-)

Just another pair of cents.

Cheers,
Felix

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

On Mon, Jan 23, 2012 at 5:15 AM, Felix Frank <
felix.frank@alumni.tu-berlin.de> wrote:
> Hi,
>
> On 01/20/2012 11:34 PM, Cody wrote:
> > Defining all somewhat common packages in a central location becomes
> > unrealistic when you no longer "control" the code that is in
every
> > module you use.  If you obtain five modules from the forge and they
> > all require a specific package and so all define that package your not
> > going to convince, nor is it a good design to require everyone to move
> > the package definitions from that collection of modules.  They need to
> > function as a collection out of the box.
>
> Agreed. How can this be accomplished?
>
Felix, could you take this to a new thread please? I''d really like to
keep
this one focused on the topic at hand if possible :)


>
> Perhaps there needs to be some kind of "Forge common" module that
by
> policy can only ever declare virtual resources (packages are a prominent
> example).
> A user who wishes to retain the capability of using modules from the
> Forge would be required to install this common module, and replace their
> own resource declarations with realizations of the common resources.
> For this to work, it''s definitely a plus that you can override
> attributes in collections:
> Package<| title == "apache2": |> { ensure =>
"2.2.12" }
> ...although that does bear some caveats. Does this still work in recent
> versions?
>
> If we can take this for granted, all Forge modules can adhere to that
> same standard.
>
> Yes, it''s quite a hassle.
>
> No, I didn''t think this through very thoroughly ;-)
>
> Just another pair of cents.
>
> Cheers,
> Felix
>
> --
> You received this message because you are subscribed to the Google Groups
> "Puppet Users" group.
> To post to this group, send email to puppet-users@googlegroups.com.
> To unsubscribe from this group, send email to
> puppet-users+unsubscribe@googlegroups.com.
> For more options, visit this group at
> http://groups.google.com/group/puppet-users?hl=en.
>
>

-- 
Nigel Kersten
Product Manager, Puppet Labs

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

Nigel,

Just so we understand the requirements of this - what would it take to make
a function that
is usable in way that a few of us have mentioned - something that lets us
say "If a class
is included in this host, then X".  It seems like this would be desirable
functionality and so
far everything I''ve heard has been a clumsy workaround for this base
kind
of functionality.

What I don''t understand is why defined creates a parse order dependency
and
why it''s not
able to search through the entire catalog to see if the class is defined.
 This is totally my
lack of understanding of the code shining through but what stops this being
possible?

Is it just the case that you don''t want to rely on a class in the
future
has hasn''t yet been
processed because it might fail and therefore, for the purposes of puppet,
not be defined?

On Mon, Jan 23, 2012 at 7:57 PM, Nigel Kersten <nigel@puppetlabs.com>
wrote:
>
>
> On Mon, Jan 23, 2012 at 5:15 AM, Felix Frank <
> felix.frank@alumni.tu-berlin.de> wrote:
>
>> Hi,
>>
>> On 01/20/2012 11:34 PM, Cody wrote:
>> > Defining all somewhat common packages in a central location
becomes
>> > unrealistic when you no longer "control" the code that
is in every
>> > module you use.  If you obtain five modules from the forge and
they
>> > all require a specific package and so all define that package your
not
>> > going to convince, nor is it a good design to require everyone to
move
>> > the package definitions from that collection of modules.  They
need to
>> > function as a collection out of the box.
>>
>> Agreed. How can this be accomplished?
>>
>
> Felix, could you take this to a new thread please? I''d really like
to keep
> this one focused on the topic at hand if possible :)
>
>
>
>>
>> Perhaps there needs to be some kind of "Forge common" module
that by
>> policy can only ever declare virtual resources (packages are a
prominent
>> example).
>> A user who wishes to retain the capability of using modules from the
>> Forge would be required to install this common module, and replace
their
>> own resource declarations with realizations of the common resources.
>> For this to work, it''s definitely a plus that you can override
>> attributes in collections:
>> Package<| title == "apache2": |> { ensure =>
"2.2.12" }
>> ...although that does bear some caveats. Does this still work in recent
>> versions?
>>
>> If we can take this for granted, all Forge modules can adhere to that
>> same standard.
>>
>> Yes, it''s quite a hassle.
>>
>> No, I didn''t think this through very thoroughly ;-)
>>
>> Just another pair of cents.
>>
>> Cheers,
>> Felix
>>
>> --
>> You received this message because you are subscribed to the Google
Groups
>> "Puppet Users" group.
>> To post to this group, send email to puppet-users@googlegroups.com.
>> To unsubscribe from this group, send email to
>> puppet-users+unsubscribe@googlegroups.com.
>> For more options, visit this group at
>> http://groups.google.com/group/puppet-users?hl=en.
>>
>>
>
>
> --
> Nigel Kersten
> Product Manager, Puppet Labs
>
>
>  --
> You received this message because you are subscribed to the Google Groups
> "Puppet Users" group.
> To post to this group, send email to puppet-users@googlegroups.com.
> To unsubscribe from this group, send email to
> puppet-users+unsubscribe@googlegroups.com.
> For more options, visit this group at
> http://groups.google.com/group/puppet-users?hl=en.
>
-- 
You received this message because you are subscribed to the Google Groups
"Puppet Users" group.
To post to this group, send email to puppet-users@googlegroups.com.
To unsubscribe from this group, send email to
puppet-users+unsubscribe@googlegroups.com.
For more options, visit this group at
http://groups.google.com/group/puppet-users?hl=en.

Hi,

there was a discussion in the "can we deprecate defined() in Telly"
thread about how we can even begin to design Forge modules without it.

A recurring problem is that multiple modules rely on certain packages,
and there is no good model (yet) to unite their resource declarations.
Therefore it''s a common (although imho disgusting) workaround to do
things like
if !defined(Package[foo]) { package { "foo": ensure => installed }
}

On 01/20/2012 11:34 PM, Cody wrote:
> Defining all somewhat common packages in a central location becomes
> unrealistic when you no longer "control" the code that is in
every
> module you use.  If you obtain five modules from the forge and they
> all require a specific package and so all define that package your not
> going to convince, nor is it a good design to require everyone to move
> the package definitions from that collection of modules.  They need to
> function as a collection out of the box.
Agreed. How can this be accomplished?

Perhaps there needs to be some kind of "Forge common" module that by
policy can only ever declare virtual resources (packages are a prominent
example).
A user who wishes to retain the capability of using modules from the
Forge would be required to install this common module, and replace their
own resource declarations with realizations of the common resources.
For this to work, it''s definitely a plus that you can override
attributes in collections:
Package<| title == "apache2": |> { ensure =>
"2.2.12" }
...although that does bear some caveats. Does this still work in recent
versions?

If we can take this for granted, all Forge modules can adhere to that
same standard.

This is a rough sketch of how things might possibly work, and surely has
lots of wrinkles of its own. Still, I''m quite sure we need a proper way
to rid ourselves of the horror that is the parse order dependent check
for defined resources ;-)

Cheers,
Felix

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

On 01/24/2012 01:57 AM, Nigel Kersten wrote:
> Felix, could you take this to a new thread please? I''d really like
to
> keep this one focused on the topic at hand if possible :)
I''d had loved to, but I''m apparently too dumb to use E-Mail.
Let''s try
again ;(

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

I was thinking more in terms of an exception handler:

package { "foo": ensure => installed, exceptDefined => skip}

Or something of that nature.  This could also be used in other
situations where you want to bypass default behaviors.  We''ve seen
situations where users want to apply a file if it exists in the module
but otherwise proceed without errors.  This could be done like so:

file {"$foo": ensure => present, source =>
"${foo}.txt", exceptAbsent => skip}


On Tue, Jan 24, 2012 at 1:28 AM, Felix Frank
<felix.frank@alumni.tu-berlin.de> wrote:
> Hi,
>
> there was a discussion in the "can we deprecate defined() in
Telly"
> thread about how we can even begin to design Forge modules without it.
>
> A recurring problem is that multiple modules rely on certain packages,
> and there is no good model (yet) to unite their resource declarations.
> Therefore it''s a common (although imho disgusting) workaround to
do
> things like
> if !defined(Package[foo]) { package { "foo": ensure =>
installed } }
>
> On 01/20/2012 11:34 PM, Cody wrote:
>> Defining all somewhat common packages in a central location becomes
>> unrealistic when you no longer "control" the code that is in
every
>> module you use.  If you obtain five modules from the forge and they
>> all require a specific package and so all define that package your not
>> going to convince, nor is it a good design to require everyone to move
>> the package definitions from that collection of modules.  They need to
>> function as a collection out of the box.
>
> Agreed. How can this be accomplished?
>
> Perhaps there needs to be some kind of "Forge common" module that
by
> policy can only ever declare virtual resources (packages are a prominent
> example).
> A user who wishes to retain the capability of using modules from the
> Forge would be required to install this common module, and replace their
> own resource declarations with realizations of the common resources.
> For this to work, it''s definitely a plus that you can override
> attributes in collections:
> Package<| title == "apache2": |> { ensure =>
"2.2.12" }
> ...although that does bear some caveats. Does this still work in recent
> versions?
>
> If we can take this for granted, all Forge modules can adhere to that
> same standard.
>
> This is a rough sketch of how things might possibly work, and surely has
> lots of wrinkles of its own. Still, I''m quite sure we need a
proper way
> to rid ourselves of the horror that is the parse order dependent check
> for defined resources ;-)
>
> Cheers,
> Felix
>
> --
> You received this message because you are subscribed to the Google Groups
"Puppet Users" group.
> To post to this group, send email to puppet-users@googlegroups.com.
> To unsubscribe from this group, send email to
puppet-users+unsubscribe@googlegroups.com.
> For more options, visit this group at
http://groups.google.com/group/puppet-users?hl=en.
>
-- 
You received this message because you are subscribed to the Google Groups
"Puppet Users" group.
To post to this group, send email to puppet-users@googlegroups.com.
To unsubscribe from this group, send email to
puppet-users+unsubscribe@googlegroups.com.
For more options, visit this group at
http://groups.google.com/group/puppet-users?hl=en.

On Tue, Jan 24, 2012 at 1:28 AM, Felix Frank <
felix.frank@alumni.tu-berlin.de> wrote:
> Hi,
>
> there was a discussion in the "can we deprecate defined() in
Telly"
> thread about how we can even begin to design Forge modules without it.
>
> A recurring problem is that multiple modules rely on certain packages,
> and there is no good model (yet) to unite their resource declarations.
> Therefore it''s a common (although imho disgusting) workaround to
do
> things like
> if !defined(Package[foo]) { package { "foo": ensure =>
installed } }
>
> On 01/20/2012 11:34 PM, Cody wrote:
> > Defining all somewhat common packages in a central location becomes
> > unrealistic when you no longer "control" the code that is in
every
> > module you use.  If you obtain five modules from the forge and they
> > all require a specific package and so all define that package your not
> > going to convince, nor is it a good design to require everyone to move
> > the package definitions from that collection of modules.  They need to
> > function as a collection out of the box.
>
> Agreed. How can this be accomplished?
>
> Perhaps there needs to be some kind of "Forge common" module that
by
> policy can only ever declare virtual resources (packages are a prominent
>
Until there is a way to distinguish between collection of regular versus
virtual resources, I hope that we don''t do anything that advocates the
usage of virtual resources.

The problem is that collections (like below) meant to
establish multiple dependencies unfortunately have the side effect of
realizing virtual resources.

Class[''apt''] -> Package<| |>

> example).
> A user who wishes to retain the capability of using modules from the
> Forge would be required to install this common module, and replace their
> own resource declarations with realizations of the common resources.
> For this to work, it''s definitely a plus that you can override
> attributes in collections:
> Package<| title == "apache2": |> { ensure =>
"2.2.12" }
> ...although that does bear some caveats. Does this still work in recent
> versions?
>
> If we can take this for granted, all Forge modules can adhere to that
> same standard.
>
> This is a rough sketch of how things might possibly work, and surely has
> lots of wrinkles of its own. Still, I''m quite sure we need a
proper way
> to rid ourselves of the horror that is the parse order dependent check
> for defined resources ;-)
>
> Cheers,
> Felix
>
> --
> You received this message because you are subscribed to the Google Groups
> "Puppet Users" group.
> To post to this group, send email to puppet-users@googlegroups.com.
> To unsubscribe from this group, send email to
> puppet-users+unsubscribe@googlegroups.com.
> For more options, visit this group at
> http://groups.google.com/group/puppet-users?hl=en.
>
>
-- 
You received this message because you are subscribed to the Google Groups
"Puppet Users" group.
To post to this group, send email to puppet-users@googlegroups.com.
To unsubscribe from this group, send email to
puppet-users+unsubscribe@googlegroups.com.
For more options, visit this group at
http://groups.google.com/group/puppet-users?hl=en.

I don''t have a solution but I do want to chime in and state that
modules
that
exist just to collect together virtual resources is a horrible thing.
 We''re just
talking about working around the way Puppet works right now rather than
defining what the end solution should be.

I would be happiest with some kind of thing that can handle multiple
versions
of the same resource and work out a way to apply them as one entry.  Like
others suggested something that can merge all the unique attributes together
for one entry.  After all, if they''ve been included on the host all of
those attributes
should apply so I can''t see a dangerous downside to this.

It has to be better than trying to organize things so you only use a
resource
once and having all kinds of tricks and complicated workarounds.  I
don''t
want
my modules to have to rely on some weird generic module full of virtual
resources
just to use a package without clashing into a module written by a coworker.
 I
think modules should be capable, if possible, of standing alone, and this
is ruining
that basic concept.

On Wed, Jan 25, 2012 at 12:12 PM, Dan Bode <dan@puppetlabs.com> wrote:
>
>
> On Tue, Jan 24, 2012 at 1:28 AM, Felix Frank <
> felix.frank@alumni.tu-berlin.de> wrote:
>
>> Hi,
>>
>> there was a discussion in the "can we deprecate defined() in
Telly"
>> thread about how we can even begin to design Forge modules without it.
>>
>> A recurring problem is that multiple modules rely on certain packages,
>> and there is no good model (yet) to unite their resource declarations.
>> Therefore it''s a common (although imho disgusting) workaround
to do
>> things like
>> if !defined(Package[foo]) { package { "foo": ensure =>
installed } }
>>
>> On 01/20/2012 11:34 PM, Cody wrote:
>> > Defining all somewhat common packages in a central location
becomes
>> > unrealistic when you no longer "control" the code that
is in every
>> > module you use.  If you obtain five modules from the forge and
they
>> > all require a specific package and so all define that package your
not
>> > going to convince, nor is it a good design to require everyone to
move
>> > the package definitions from that collection of modules.  They
need to
>> > function as a collection out of the box.
>>
>> Agreed. How can this be accomplished?
>>
>> Perhaps there needs to be some kind of "Forge common" module
that by
>> policy can only ever declare virtual resources (packages are a
prominent
>>
>
> Until there is a way to distinguish between collection of regular versus
> virtual resources, I hope that we don''t do anything that advocates
the
> usage of virtual resources.
>
> The problem is that collections (like below) meant to
> establish multiple dependencies unfortunately have the side effect of
> realizing virtual resources.
>
> Class[''apt''] -> Package<| |>
>
>
>> example).
>> A user who wishes to retain the capability of using modules from the
>> Forge would be required to install this common module, and replace
their
>> own resource declarations with realizations of the common resources.
>> For this to work, it''s definitely a plus that you can override
>> attributes in collections:
>> Package<| title == "apache2": |> { ensure =>
"2.2.12" }
>> ...although that does bear some caveats. Does this still work in recent
>> versions?
>>
>> If we can take this for granted, all Forge modules can adhere to that
>> same standard.
>>
>> This is a rough sketch of how things might possibly work, and surely
has
>> lots of wrinkles of its own. Still, I''m quite sure we need a
proper way
>> to rid ourselves of the horror that is the parse order dependent check
>> for defined resources ;-)
>>
>> Cheers,
>> Felix
>>
>> --
>> You received this message because you are subscribed to the Google
Groups
>> "Puppet Users" group.
>> To post to this group, send email to puppet-users@googlegroups.com.
>> To unsubscribe from this group, send email to
>> puppet-users+unsubscribe@googlegroups.com.
>> For more options, visit this group at
>> http://groups.google.com/group/puppet-users?hl=en.
>>
>>
>  --
> You received this message because you are subscribed to the Google Groups
> "Puppet Users" group.
> To post to this group, send email to puppet-users@googlegroups.com.
> To unsubscribe from this group, send email to
> puppet-users+unsubscribe@googlegroups.com.
> For more options, visit this group at
> http://groups.google.com/group/puppet-users?hl=en.
>
-- 
You received this message because you are subscribed to the Google Groups
"Puppet Users" group.
To post to this group, send email to puppet-users@googlegroups.com.
To unsubscribe from this group, send email to
puppet-users+unsubscribe@googlegroups.com.
For more options, visit this group at
http://groups.google.com/group/puppet-users?hl=en.

On Wed, Jan 25, 2012 at 9:59 AM, Ashley Penney <apenney@gmail.com> wrote:
> I don''t have a solution but I do want to chime in and state that
modules
> that
> exist just to collect together virtual resources is a horrible thing.
>  We''re just
> talking about working around the way Puppet works right now rather than
> defining what the end solution should be.
>
Absolutely right Ashley.

Please lets focus the conversation here on how things *should* be.

>
> I would be happiest with some kind of thing that can handle multiple
> versions
> of the same resource and work out a way to apply them as one entry.  Like
> others suggested something that can merge all the unique attributes
> together
> for one entry.  After all, if they''ve been included on the host
all of
> those attributes
> should apply so I can''t see a dangerous downside to this.
>
> It has to be better than trying to organize things so you only use a
> resource
> once and having all kinds of tricks and complicated workarounds.  I
don''t
> want
> my modules to have to rely on some weird generic module full of virtual
> resources
> just to use a package without clashing into a module written by a
> coworker.  I
> think modules should be capable, if possible, of standing alone, and this
> is ruining
> that basic concept.
>
> On Wed, Jan 25, 2012 at 12:12 PM, Dan Bode <dan@puppetlabs.com>
wrote:
>
>>
>>
>> On Tue, Jan 24, 2012 at 1:28 AM, Felix Frank <
>> felix.frank@alumni.tu-berlin.de> wrote:
>>
>>> Hi,
>>>
>>> there was a discussion in the "can we deprecate defined() in
Telly"
>>> thread about how we can even begin to design Forge modules without
it.
>>>
>>> A recurring problem is that multiple modules rely on certain
packages,
>>> and there is no good model (yet) to unite their resource
declarations.
>>> Therefore it''s a common (although imho disgusting)
workaround to do
>>> things like
>>> if !defined(Package[foo]) { package { "foo": ensure =>
installed } }
>>>
>>> On 01/20/2012 11:34 PM, Cody wrote:
>>> > Defining all somewhat common packages in a central location
becomes
>>> > unrealistic when you no longer "control" the code
that is in every
>>> > module you use.  If you obtain five modules from the forge and
they
>>> > all require a specific package and so all define that package
your not
>>> > going to convince, nor is it a good design to require everyone
to move
>>> > the package definitions from that collection of modules.  They
need to
>>> > function as a collection out of the box.
>>>
>>> Agreed. How can this be accomplished?
>>>
>>> Perhaps there needs to be some kind of "Forge common"
module that by
>>> policy can only ever declare virtual resources (packages are a
prominent
>>>
>>
>> Until there is a way to distinguish between collection of regular
versus
>> virtual resources, I hope that we don''t do anything that
advocates the
>> usage of virtual resources.
>>
>> The problem is that collections (like below) meant to
>> establish multiple dependencies unfortunately have the side effect of
>> realizing virtual resources.
>>
>> Class[''apt''] -> Package<| |>
>>
>>
>>> example).
>>> A user who wishes to retain the capability of using modules from
the
>>> Forge would be required to install this common module, and replace
their
>>> own resource declarations with realizations of the common
resources.
>>> For this to work, it''s definitely a plus that you can
override
>>> attributes in collections:
>>> Package<| title == "apache2": |> { ensure =>
"2.2.12" }
>>> ...although that does bear some caveats. Does this still work in
recent
>>> versions?
>>>
>>> If we can take this for granted, all Forge modules can adhere to
that
>>> same standard.
>>>
>>> This is a rough sketch of how things might possibly work, and
surely has
>>> lots of wrinkles of its own. Still, I''m quite sure we need
a proper way
>>> to rid ourselves of the horror that is the parse order dependent
check
>>> for defined resources ;-)
>>>
>>> Cheers,
>>> Felix
>>>
>>> --
>>> You received this message because you are subscribed to the Google
>>> Groups "Puppet Users" group.
>>> To post to this group, send email to puppet-users@googlegroups.com.
>>> To unsubscribe from this group, send email to
>>> puppet-users+unsubscribe@googlegroups.com.
>>> For more options, visit this group at
>>> http://groups.google.com/group/puppet-users?hl=en.
>>>
>>>
>>  --
>> You received this message because you are subscribed to the Google
Groups
>> "Puppet Users" group.
>> To post to this group, send email to puppet-users@googlegroups.com.
>> To unsubscribe from this group, send email to
>> puppet-users+unsubscribe@googlegroups.com.
>> For more options, visit this group at
>> http://groups.google.com/group/puppet-users?hl=en.
>>
>
>  --
> You received this message because you are subscribed to the Google Groups
> "Puppet Users" group.
> To post to this group, send email to puppet-users@googlegroups.com.
> To unsubscribe from this group, send email to
> puppet-users+unsubscribe@googlegroups.com.
> For more options, visit this group at
> http://groups.google.com/group/puppet-users?hl=en.
>


-- 
Nigel Kersten
Product Manager, Puppet Labs

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

On 01/25/2012 06:59 PM, Ashley Penney wrote:
> After all, if they''ve been included on the host all of those
attributes
> should apply so I can''t see a dangerous downside to this.
Nan provided some killer arguments (imho) in the proper cross-dependency
thread.

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

On Thu, Jan 19, 2012 at 9:18 AM, Nigel Kersten <nigel@puppetlabs.com>
wrote:
> I''m looking for strong opinions on whether we should or
shouldn''t deprecate
> the defined() function for Telly, the next major Puppet release this year.
>
> jcbollinger put it quite well in another thread:
>
> "Use of the "defined" function introduces a parse-order
dependency, and the
> additional work you need to do to ensure that that dependency is always
> fulfilled overcomes any simplicity advantage that might otherwise
exist."
I''m coming into this thread a bit late so I''m going to take
the
strategy of replying to the OP and pasting a lot of the major comments
about the related issues into this reply and then addressing those
directly, inline.

The TL;DR version is that I strongly believe defined() is an
anti-pattern and should be removed from Puppet core.

For those who would still like to have it available I think it should
be added to the stdlib module as two new functions: already_defined()
and already_declared().  The already_ prefix is to clearly communicate
these are parser-order dependent functions and they _do not_ operate
on the final catalog itself.

Finally, for those who want 100% backwards compatibility, we publish a
new module in addition to, and compatible with, stdlib that provides
the existing behavior of defined().


And now for the longer version of why I have this strong opinion:

Throughout the replies in this thread, many people are using defined()
to test of a resource or class has been added to the catalog and if
not, add a resource with the same type and title to the catalog.  I''m
going to call this the "if not defined then declare" anti-pattern.
I''ll explain why it''s an anti-pattern at the conclusion.  I
hope
showing the pattern to be an anti-pattern will be sufficient to show
the pattern should be avoided when publishing modules.

Nick Fagerlund said:
> Defined() doesn''t suck! It''s a 100% reliable way to check
what classes and defined types are available to the autoloader.
This is an accidental feature and orthogonal to the intent of defined.
 There''s also not much evidence that the Puppet community is
leveraging defined() in this manner.  Finally, we should implement a
first-class and supported function to provide this feature in a clear
way.  is_available() perhaps.

Ashley Penney said:
> if ! defined(Mysql_user ["${user}@${host}"]) { mysql_user {
"${user}@${host}": ... } }
This is the "if not defined then declare" anti-pattern.

Felix Frank said:
> If your master processes this before the *other* declaration of that
mysql_user{}, you''re back to square one and get errors about multiple
resource declaration.
This is part of why "if not defined then declare" is an ant-pattern.

Nan Liu said:
> "We should at least differentiate defined vs. declared"
Yes, certainly.  We also need to keep in mind that a function can only
operate on a partial catalog.  A function can never operate on a
complete catalog.  Therefore, we need to think of these as "defined
_yet_" and "declared _yet_" with respect to parse order.

My proposal here is to add already_defined() and already_declared() to
stdlib.  The names are only an example, I don''t really care what
they''re called so long as their names take into account the fact that
a function can only operate on a partially compiled catalog.

Dan Bode said:
> * static resources in a defined resource type (avoids having to use classes
to store all static dependencies)

This looks like the anti-pattern, but is not IFF the static resource
being declared is entirely "private" to the module.  If the static
resource is something more "public" or "common" like a
resource for a
database user/password then this _is_ the anti-pattern.
> * the big reason I keep on leaning on it is for package dependencies.
Using defined() for package dependencies is always an anti-pattern.
Package dependencies are always "common" and therefore two modules
that have the same package dependency need to express this as a third
class.  If they don''t, then catalogs will have the same resource
contained in different classes from compilation to compilation because
of the parse order nature of functions.
> How about deprecating defined?(Type[''title'']), but
allowing it to accept a resource hash?
For the case of package dependencies, moving the dependencies to
another module / class is much more preferable to this pattern because
there is a high probability other modules will need the dependencies
satisfied as well.  If the dependencies are not in their own module or
class then the modules that share common package dependencies must
repeat this anti-pattern and must know the exact way in which the
package resource has been declared.

Felix Frank wrote:
> ... endorsing defined() abuse in such a manner will lead to bad (and ugly)
code all over the place ...
I share this opinion.  Strongly.  While the if not defined then
declare anti-pattern works, and works well for the internal
organization of a set of modules by a single author it has the
undesirable side effect of requiring other module authors who depend
on the same resources to repeat the anti-pattern.

jcbollinger wrote (in response to, "the big reason I keep on leaning
on it is for package dependencies"):
> You describe one of the core use cases for virtual resources.
Yes, but unfortunately virtual resources are difficult to leverage in
a module re-use setting because of their global state.  I consider
virtual resources to be a feature for "end users" rather than a tool
for "module authors" who are publishing to the world.

For example, I expect some users to write Package <| |> { ensure =>
present } which may wreck havoc with virtual resources I''ve declared
for "internal use" by my modules that I''m publishing.

Cody said:
> Defining (virtual resources for) all somewhat common packages in a central
location becomes unrealistic when you no longer "control" the code
that is in every module you use.
Yes, this is why I consider virtual resources to be a tool for
end-users and not module authors.

Aaron Grewell wrote:
> What makes defined() so different from the code that implements require?
What makes it so different is that defined() is used primarily with
resources, which cannot be declared multiple times, while require() is
used primarily with classes which can be declared multiple times but
are only ever added to the catalog once.
> Shouldn''t "if  not defined" be the same as "if a
require would fail"?  That seems to be what people are expecting, why not
give it to them?
It basically is.  It''s an anti-pattern because even though
that''s what
people expect, the behavior of the system is to change behavior from
catalog compilation to catalog compilation.  People don''t expect this
aspect, hence why I strongly believe it satisfies the, "Some repeated
pattern of action, process or structure that initially appears to be
beneficial, but ultimately produces more bad consequences than
beneficial results" element.

Felix Frank wrote:
> Perhaps there needs to be some kind of "Forge common" module that
by policy can only ever declare virtual resources (packages are a prominent
example).
This is a good idea, but untenable.  We can''t require new Puppet users
to understand this level of complexity and intricacy in order to use
modules from the Forge.

Ashley Penney wrote:
> ... why does defined create a parse order dependency and why is it not able
to search through the entire catalog to see if the class is defined?
defined() is parse order dependent because functions are only ever
evaluated during the process of _compiling_ a catalog.  They''re never
evaluated _after_ a catalog is compiled.  Therefore, the idea of the,
"entire catalog" doesn''t really exist when we''re
talking about parser
functions because we only have access to, "the catalog we''ve built
up
(compiled) _so far_."

Hope this helps.

Dan Bode said:
> Until there is a way to distinguish between collection of regular versus
virtual resources, I hope that we don''t do anything that advocates the
usage of virtual resources.
This is my opinion as well.  We, as module authors and publishers,
can''t use virtual resources as a tool in our toolbox for this reason.

Nigel Kersten said:
> Please lets focus the conversation here on how things *should* be.
Yep.

We should be able to declare dependencies of a module in a way that is
deterministic and results in the same resources being added to the
same catalog given the same set of manifests.  We can, today, using
what I consider the "third party" module pattern.  That is, if you
have a dependency like Java that another module is likely to have then
Java should be a third party to your module and the other module.  We
can do this today which satisfies the, "A refactored solution exists
that is clearly documented, proven in actual practice and repeatable."
element of an Anti-Pattern.

For the original question, we also have a place to move the previous
functionality to, the stdlib module.  Therefore, I think we should
remove declared() and defined() from Puppet core entirely for Telly
and add more clear replacements to stdlib.  Finally, add
backwards-compatibility to another module and publish it.

-Jeff

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

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Jeff,

Thanks for the concise write-up, it was really helpful for parsing through all
of the different arguments.

I would like to second the idea that Aaron Grewell had earlier that some sort of
exception handling be allowed and that some other method for doing
the "if I have class A, then do something with class A, otherwise,
don''t" scenario is provided. Adding the methods to stdlib would
work it just needs
to be available before Telly drops.

I mentioned in a previous thread that I don''t see an issue with having
multiple identical resources compiled across the code base and I''d like
to add
that to this thread since it''s related.

class a {
  package { ''foo'': ensure => ''present'' }
}

class b {
  package { ''foo'': ensure => ''present'' }
}

include ''a''
include ''b''

Should work. However, if the two resources differ, this should be a compile
error. In a perfect world, you wouldn''t have this issue, but it
shouldn''t
be an error since you''re applying identical code.

I would also like to make a related request...

Whenever the Puppet core is to change in a way that will break existing code, I
would like a utility to parse my entire code base, whether or not is
is being actively used, to identify potential areas of concern. Right now, there
are deprecation warnings, but those are only output when the code is
run, not if the code is simply in the code base.

puppet-lint helps with some of this and is appreciated but not quite adequate
and the edge cases are difficult to catch.

It feels like Puppet is working its way toward a two pass compile, one for
static code portions and one for dynamic portions. While potentially less
efficient, it would add greater room for the flexibility that people seem to
want overall.

Thanks,

Trevor


On 01/25/2012 03:17 PM, Jeff McCune wrote:
> On Thu, Jan 19, 2012 at 9:18 AM, Nigel Kersten <nigel@puppetlabs.com>
wrote:
>> I''m looking for strong opinions on whether we should or
shouldn''t deprecate
>> the defined() function for Telly, the next major Puppet release this
year.
>>
>> jcbollinger put it quite well in another thread:
>>
>> "Use of the "defined" function introduces a parse-order
dependency, and the
>> additional work you need to do to ensure that that dependency is always
>> fulfilled overcomes any simplicity advantage that might otherwise
exist."
> 
> I''m coming into this thread a bit late so I''m going to
take the
> strategy of replying to the OP and pasting a lot of the major comments
> about the related issues into this reply and then addressing those
> directly, inline.
> 
> The TL;DR version is that I strongly believe defined() is an
> anti-pattern and should be removed from Puppet core.
> 
> For those who would still like to have it available I think it should
> be added to the stdlib module as two new functions: already_defined()
> and already_declared().  The already_ prefix is to clearly communicate
> these are parser-order dependent functions and they _do not_ operate
> on the final catalog itself.
> 
> Finally, for those who want 100% backwards compatibility, we publish a
> new module in addition to, and compatible with, stdlib that provides
> the existing behavior of defined().
> 
> 
> And now for the longer version of why I have this strong opinion:
> 
> Throughout the replies in this thread, many people are using defined()
> to test of a resource or class has been added to the catalog and if
> not, add a resource with the same type and title to the catalog. 
I''m
> going to call this the "if not defined then declare"
anti-pattern.
> I''ll explain why it''s an anti-pattern at the conclusion. 
I hope
> showing the pattern to be an anti-pattern will be sufficient to show
> the pattern should be avoided when publishing modules.
> 
> Nick Fagerlund said:
>> Defined() doesn''t suck! It''s a 100% reliable way to
check what classes and defined types are available to the autoloader.
> 
> This is an accidental feature and orthogonal to the intent of defined.
>  There''s also not much evidence that the Puppet community is
> leveraging defined() in this manner.  Finally, we should implement a
> first-class and supported function to provide this feature in a clear
> way.  is_available() perhaps.
> 
> Ashley Penney said:
>> if ! defined(Mysql_user ["${user}@${host}"]) { mysql_user {
"${user}@${host}": ... } }
> 
> This is the "if not defined then declare" anti-pattern.
> 
> Felix Frank said:
>> If your master processes this before the *other* declaration of that
mysql_user{}, you''re back to square one and get errors about multiple
resource declaration.
> 
> This is part of why "if not defined then declare" is an
ant-pattern.
> 
> Nan Liu said:
>> "We should at least differentiate defined vs. declared"
> 
> Yes, certainly.  We also need to keep in mind that a function can only
> operate on a partial catalog.  A function can never operate on a
> complete catalog.  Therefore, we need to think of these as "defined
> _yet_" and "declared _yet_" with respect to parse order.
> 
> My proposal here is to add already_defined() and already_declared() to
> stdlib.  The names are only an example, I don''t really care what
> they''re called so long as their names take into account the fact
that
> a function can only operate on a partially compiled catalog.
> 
> Dan Bode said:
>> * static resources in a defined resource type (avoids having to use
classes
> to store all static dependencies)
> 
> This looks like the anti-pattern, but is not IFF the static resource
> being declared is entirely "private" to the module.  If the
static
> resource is something more "public" or "common" like a
resource for a
> database user/password then this _is_ the anti-pattern.
> 
>> * the big reason I keep on leaning on it is for package dependencies.
> 
> Using defined() for package dependencies is always an anti-pattern.
> Package dependencies are always "common" and therefore two
modules
> that have the same package dependency need to express this as a third
> class.  If they don''t, then catalogs will have the same resource
> contained in different classes from compilation to compilation because
> of the parse order nature of functions.
> 
>> How about deprecating defined?(Type[''title'']), but
allowing it to accept a resource hash?
> 
> For the case of package dependencies, moving the dependencies to
> another module / class is much more preferable to this pattern because
> there is a high probability other modules will need the dependencies
> satisfied as well.  If the dependencies are not in their own module or
> class then the modules that share common package dependencies must
> repeat this anti-pattern and must know the exact way in which the
> package resource has been declared.
> 
> Felix Frank wrote:
> 
>> ... endorsing defined() abuse in such a manner will lead to bad (and
ugly) code all over the place ...
> 
> I share this opinion.  Strongly.  While the if not defined then
> declare anti-pattern works, and works well for the internal
> organization of a set of modules by a single author it has the
> undesirable side effect of requiring other module authors who depend
> on the same resources to repeat the anti-pattern.
> 
> jcbollinger wrote (in response to, "the big reason I keep on leaning
> on it is for package dependencies"):
> 
>> You describe one of the core use cases for virtual resources.
> 
> Yes, but unfortunately virtual resources are difficult to leverage in
> a module re-use setting because of their global state.  I consider
> virtual resources to be a feature for "end users" rather than a
tool
> for "module authors" who are publishing to the world.
> 
> For example, I expect some users to write Package <| |> { ensure
=>
> present } which may wreck havoc with virtual resources I''ve
declared
> for "internal use" by my modules that I''m publishing.
> 
> Cody said:
>> Defining (virtual resources for) all somewhat common packages in a
central location becomes unrealistic when you no longer "control" the
code that is in every module you use.
> 
> Yes, this is why I consider virtual resources to be a tool for
> end-users and not module authors.
> 
> Aaron Grewell wrote:
> 
>> What makes defined() so different from the code that implements
require?
> 
> What makes it so different is that defined() is used primarily with
> resources, which cannot be declared multiple times, while require() is
> used primarily with classes which can be declared multiple times but
> are only ever added to the catalog once.
> 
>> Shouldn''t "if  not defined" be the same as "if
a require would fail"?  That seems to be what people are expecting, why not
give it to them?
> 
> It basically is.  It''s an anti-pattern because even though
that''s what
> people expect, the behavior of the system is to change behavior from
> catalog compilation to catalog compilation.  People don''t expect
this
> aspect, hence why I strongly believe it satisfies the, "Some repeated
> pattern of action, process or structure that initially appears to be
> beneficial, but ultimately produces more bad consequences than
> beneficial results" element.
> 
> Felix Frank wrote:
> 
>> Perhaps there needs to be some kind of "Forge common" module
that by policy can only ever declare virtual resources (packages are a prominent
example).
> 
> This is a good idea, but untenable.  We can''t require new Puppet
users
> to understand this level of complexity and intricacy in order to use
> modules from the Forge.
> 
> Ashley Penney wrote:
> 
>> ... why does defined create a parse order dependency and why is it not
able to search through the entire catalog to see if the class is defined?
> 
> defined() is parse order dependent because functions are only ever
> evaluated during the process of _compiling_ a catalog.  They''re
never
> evaluated _after_ a catalog is compiled.  Therefore, the idea of the,
> "entire catalog" doesn''t really exist when
we''re talking about parser
> functions because we only have access to, "the catalog we''ve
built up
> (compiled) _so far_."
> 
> Hope this helps.
> 
> Dan Bode said:
> 
>> Until there is a way to distinguish between collection of regular
versus virtual resources, I hope that we don''t do anything that
advocates the usage of virtual resources.
> 
> This is my opinion as well.  We, as module authors and publishers,
> can''t use virtual resources as a tool in our toolbox for this
reason.
> 
> Nigel Kersten said:
> 
>> Please lets focus the conversation here on how things *should* be.
> 
> Yep.
> 
> We should be able to declare dependencies of a module in a way that is
> deterministic and results in the same resources being added to the
> same catalog given the same set of manifests.  We can, today, using
> what I consider the "third party" module pattern.  That is, if
you
> have a dependency like Java that another module is likely to have then
> Java should be a third party to your module and the other module.  We
> can do this today which satisfies the, "A refactored solution exists
> that is clearly documented, proven in actual practice and repeatable."
> element of an Anti-Pattern.
> 
> For the original question, we also have a place to move the previous
> functionality to, the stdlib module.  Therefore, I think we should
> remove declared() and defined() from Puppet core entirely for Telly
> and add more clear replacements to stdlib.  Finally, add
> backwards-compatibility to another module and publish it.
> 
> -Jeff
> 
- -- 
Trevor Vaughan
 Vice President, Onyx Point, Inc.
 email: tvaughan@onyxpoint.com
 phone: 410-541-ONYX (6699)
 pgp: 0x6C701E94

- -- This account not approved for unencrypted sensitive information --
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)

iQEcBAEBAgAGBQJPIKSQAAoJECNCGV1OLcypp6gIAJQNC2qENy1+tGjOCrcyiq10
ZIp5CCxS1d2Jof4bltIxKNhhiNlX1d5YSxM6epFwGiL8ZSqOx0QtE9U04DTOe2iJ
ThQ+AjePFC+8znzeTJHklUq51q2tOGBzsRjv3SyVWpd1hloOnvU1jx16a4R2oWHI
2Z/4zoCNy3bk7e2hKTCKhkBmQmmxA0H1PIW5eCTsFZEYrvp8nvOrjms7KbD5tXHE
G+aSHPYG+bvxks+lKs8pulFv7xBN1EBf+0yoe/y+iLqc6NZvlRzHHGb6VtLrA1HE
f8qPqrodEPptBtyppG9vSLIVHo9vOQP4q9J98JDLGdFdYZAzCXmQut+9WDMMN2s=3m7I
-----END PGP SIGNATURE-----

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

This is a fantastic reply and I appreciate the work you put into it.  I
have just one
question.  As it stands functions can only apply to partial catalogs and
not to the
full catalog.  Is this a fundamental design decision that cannot be
changed?  Perhaps
it would be interesting to speculate on what could be done if you had the
ability to
use the entire catalog when fully parsed.

While writing that paragraph I guess I can see how that wouldn''t work,
you''d always
have to evaluate anything that required those functions at the end of the
catalog and
if they had dependencies that would be impossible.

I still dislike the third module refactoring.  I think it removes a lot of
power of self-
contained modules and makes things significantly uglier and more difficult
when
combining modules from multiple sources.  I wish it could be solved in a
better
way within Puppet and I believe it could be with (perhaps optional) merging
of
identical resources.

But hey, I''m not the guy who has to implement this so I can appreciate
that
there
are probably all kinds of design reasons this doesn''t work.

All I know is that telling users "If you download 5 modules from puppet
forge
make sure you go through them all, extract any duplicating resources into
random modules that exist purely to allow you to realize packages" instead
leads
to a really bad user experience.

On Wed, Jan 25, 2012 at 3:17 PM, Jeff McCune <jeff@puppetlabs.com> wrote:
> On Thu, Jan 19, 2012 at 9:18 AM, Nigel Kersten <nigel@puppetlabs.com>
> wrote:
> > I''m looking for strong opinions on whether we should or
shouldn''t
> deprecate
> > the defined() function for Telly, the next major Puppet release this
> year.
> >
> > jcbollinger put it quite well in another thread:
> >
> > "Use of the "defined" function introduces a parse-order
dependency, and
> the
> > additional work you need to do to ensure that that dependency is
always
> > fulfilled overcomes any simplicity advantage that might otherwise
exist."
>
> I''m coming into this thread a bit late so I''m going to
take the
> strategy of replying to the OP and pasting a lot of the major comments
> about the related issues into this reply and then addressing those
> directly, inline.
>
> The TL;DR version is that I strongly believe defined() is an
> anti-pattern and should be removed from Puppet core.
>
> For those who would still like to have it available I think it should
> be added to the stdlib module as two new functions: already_defined()
> and already_declared().  The already_ prefix is to clearly communicate
> these are parser-order dependent functions and they _do not_ operate
> on the final catalog itself.
>
> Finally, for those who want 100% backwards compatibility, we publish a
> new module in addition to, and compatible with, stdlib that provides
> the existing behavior of defined().
>
>
> And now for the longer version of why I have this strong opinion:
>
> Throughout the replies in this thread, many people are using defined()
> to test of a resource or class has been added to the catalog and if
> not, add a resource with the same type and title to the catalog. 
I''m
> going to call this the "if not defined then declare"
anti-pattern.
> I''ll explain why it''s an anti-pattern at the conclusion. 
I hope
> showing the pattern to be an anti-pattern will be sufficient to show
> the pattern should be avoided when publishing modules.
>
> Nick Fagerlund said:
> > Defined() doesn''t suck! It''s a 100% reliable way to
check what classes
> and defined types are available to the autoloader.
>
> This is an accidental feature and orthogonal to the intent of defined.
>  There''s also not much evidence that the Puppet community is
> leveraging defined() in this manner.  Finally, we should implement a
> first-class and supported function to provide this feature in a clear
> way.  is_available() perhaps.
>
> Ashley Penney said:
> > if ! defined(Mysql_user ["${user}@${host}"]) { mysql_user {
"${user}@${host}":
> ... } }
>
> This is the "if not defined then declare" anti-pattern.
>
> Felix Frank said:
> > If your master processes this before the *other* declaration of that
> mysql_user{}, you''re back to square one and get errors about
multiple
> resource declaration.
>
> This is part of why "if not defined then declare" is an
ant-pattern.
>
> Nan Liu said:
> > "We should at least differentiate defined vs. declared"
>
> Yes, certainly.  We also need to keep in mind that a function can only
> operate on a partial catalog.  A function can never operate on a
> complete catalog.  Therefore, we need to think of these as "defined
> _yet_" and "declared _yet_" with respect to parse order.
>
> My proposal here is to add already_defined() and already_declared() to
> stdlib.  The names are only an example, I don''t really care what
> they''re called so long as their names take into account the fact
that
> a function can only operate on a partially compiled catalog.
>
> Dan Bode said:
> > * static resources in a defined resource type (avoids having to use
> classes
> to store all static dependencies)
>
> This looks like the anti-pattern, but is not IFF the static resource
> being declared is entirely "private" to the module.  If the
static
> resource is something more "public" or "common" like a
resource for a
> database user/password then this _is_ the anti-pattern.
>
> > * the big reason I keep on leaning on it is for package dependencies.
>
> Using defined() for package dependencies is always an anti-pattern.
> Package dependencies are always "common" and therefore two
modules
> that have the same package dependency need to express this as a third
> class.  If they don''t, then catalogs will have the same resource
> contained in different classes from compilation to compilation because
> of the parse order nature of functions.
>
> > How about deprecating defined?(Type[''title'']), but
allowing it to accept
> a resource hash?
>
> For the case of package dependencies, moving the dependencies to
> another module / class is much more preferable to this pattern because
> there is a high probability other modules will need the dependencies
> satisfied as well.  If the dependencies are not in their own module or
> class then the modules that share common package dependencies must
> repeat this anti-pattern and must know the exact way in which the
> package resource has been declared.
>
> Felix Frank wrote:
>
> > ... endorsing defined() abuse in such a manner will lead to bad (and
> ugly) code all over the place ...
>
> I share this opinion.  Strongly.  While the if not defined then
> declare anti-pattern works, and works well for the internal
> organization of a set of modules by a single author it has the
> undesirable side effect of requiring other module authors who depend
> on the same resources to repeat the anti-pattern.
>
> jcbollinger wrote (in response to, "the big reason I keep on leaning
> on it is for package dependencies"):
>
> > You describe one of the core use cases for virtual resources.
>
> Yes, but unfortunately virtual resources are difficult to leverage in
> a module re-use setting because of their global state.  I consider
> virtual resources to be a feature for "end users" rather than a
tool
> for "module authors" who are publishing to the world.
>
> For example, I expect some users to write Package <| |> { ensure
=>
> present } which may wreck havoc with virtual resources I''ve
declared
> for "internal use" by my modules that I''m publishing.
>
> Cody said:
> > Defining (virtual resources for) all somewhat common packages in a
> central location becomes unrealistic when you no longer "control"
the code
> that is in every module you use.
>
> Yes, this is why I consider virtual resources to be a tool for
> end-users and not module authors.
>
> Aaron Grewell wrote:
>
> > What makes defined() so different from the code that implements
require?
>
> What makes it so different is that defined() is used primarily with
> resources, which cannot be declared multiple times, while require() is
> used primarily with classes which can be declared multiple times but
> are only ever added to the catalog once.
>
> > Shouldn''t "if  not defined" be the same as "if
a require would fail"?
>  That seems to be what people are expecting, why not give it to them?
>
> It basically is.  It''s an anti-pattern because even though
that''s what
> people expect, the behavior of the system is to change behavior from
> catalog compilation to catalog compilation.  People don''t expect
this
> aspect, hence why I strongly believe it satisfies the, "Some repeated
> pattern of action, process or structure that initially appears to be
> beneficial, but ultimately produces more bad consequences than
> beneficial results" element.
>
> Felix Frank wrote:
>
> > Perhaps there needs to be some kind of "Forge common" module
that by
> policy can only ever declare virtual resources (packages are a prominent
> example).
>
> This is a good idea, but untenable.  We can''t require new Puppet
users
> to understand this level of complexity and intricacy in order to use
> modules from the Forge.
>
> Ashley Penney wrote:
>
> > ... why does defined create a parse order dependency and why is it not
> able to search through the entire catalog to see if the class is defined?
>
> defined() is parse order dependent because functions are only ever
> evaluated during the process of _compiling_ a catalog.  They''re
never
> evaluated _after_ a catalog is compiled.  Therefore, the idea of the,
> "entire catalog" doesn''t really exist when
we''re talking about parser
> functions because we only have access to, "the catalog we''ve
built up
> (compiled) _so far_."
>
> Hope this helps.
>
> Dan Bode said:
>
> > Until there is a way to distinguish between collection of regular
versus
> virtual resources, I hope that we don''t do anything that advocates
the
> usage of virtual resources.
>
> This is my opinion as well.  We, as module authors and publishers,
> can''t use virtual resources as a tool in our toolbox for this
reason.
>
> Nigel Kersten said:
>
> > Please lets focus the conversation here on how things *should* be.
>
> Yep.
>
> We should be able to declare dependencies of a module in a way that is
> deterministic and results in the same resources being added to the
> same catalog given the same set of manifests.  We can, today, using
> what I consider the "third party" module pattern.  That is, if
you
> have a dependency like Java that another module is likely to have then
> Java should be a third party to your module and the other module.  We
> can do this today which satisfies the, "A refactored solution exists
> that is clearly documented, proven in actual practice and repeatable."
> element of an Anti-Pattern.
>
> For the original question, we also have a place to move the previous
> functionality to, the stdlib module.  Therefore, I think we should
> remove declared() and defined() from Puppet core entirely for Telly
> and add more clear replacements to stdlib.  Finally, add
> backwards-compatibility to another module and publish it.
>
> -Jeff
>
> --
> You received this message because you are subscribed to the Google Groups
> "Puppet Users" group.
> To post to this group, send email to puppet-users@googlegroups.com.
> To unsubscribe from this group, send email to
> puppet-users+unsubscribe@googlegroups.com.
> For more options, visit this group at
> http://groups.google.com/group/puppet-users?hl=en.
>
>
-- 
You received this message because you are subscribed to the Google Groups
"Puppet Users" group.
To post to this group, send email to puppet-users@googlegroups.com.
To unsubscribe from this group, send email to
puppet-users+unsubscribe@googlegroups.com.
For more options, visit this group at
http://groups.google.com/group/puppet-users?hl=en.

On Wed, Jan 25, 2012 at 5:25 PM, Ashley Penney <apenney@gmail.com> wrote:
>
> All I know is that telling users "If you download 5 modules from
puppet
> forge
> make sure you go through them all, extract any duplicating resources into
> random modules that exist purely to allow you to realize packages"
instead
> leads
> to a really bad user experience.
>
Absolutely. We''re aware this is a big limitation for great
collaboration on
module content, and it needs to be addressed.

Don''t you think it makes more sense to make that part of module
metadata
rather than trying to do it inside all your manifests?

If we have:

* puppet module tool supporting dependencies and generating module metadata
* classes not loading if their dependencies aren''t satisfied (or at the
least big warnings)

do we really need defined()?


-- 
Nigel Kersten
Product Manager, Puppet Labs

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

Hi,

On 01/26/2012 02:25 AM, Ashley Penney wrote:
> This is a fantastic reply and I appreciate the work you put into it.  I
> have just one
> question.  As it stands functions can only apply to partial catalogs and
> not to the
> full catalog.  Is this a fundamental design decision that cannot be
> changed?  Perhaps
> it would be interesting to speculate on what could be done if you had
> the ability to
> use the entire catalog when fully parsed.
This goes in a similar direction as Trevor''s comment:

On 01/26/2012 01:55 AM, Trevor Vaughan wrote:
> It feels like Puppet is working its way toward a two pass compile, one
for static code portions and one for dynamic portions. While potentially
less
> efficient, it would add greater room for the flexibility that people
seem to want overall.

Please let''s NOT go down that road. You won''t get away with
two passes.
Think about it: After every pass, some if { } could switch values
depending on what defined() or some other function now returns.
The compiler would need to recursively repeat all its work (don''t even
get me started on infinite loops).
That''s another road to pain right there IMO.
> I still dislike the third module refactoring.  I think it removes a lot
> of power of self-
> contained modules and makes things significantly uglier and more
> difficult when
> combining modules from multiple sources.  I wish it could be solved in a
> better
> way within Puppet and I believe it could be with (perhaps optional)
> merging of
> identical resources.
Modules that work in and of itself are desirable, seeing as they''re
very
elegant. On the other hand, in the worst case you duplicate lots of code
(yes, package { "java": } is not a lot of code), which modules should
normally keep you from doing.
> All I know is that telling users "If you download 5 modules from
puppet
> forge
> make sure you go through them all, extract any duplicating resources into
> random modules that exist purely to allow you to realize packages"
> instead leads
> to a really bad user experience.
Uhm, what? o_O

That''s not at all what I had in mind. This is a job for *authors*, not
end users.

Thinking about other examples of similar systems (CPAN, Gems, Pear, you
name it), module dependencies are commonplace (and usually coupled with
a system that will automatically resolve them for the end user).

I stronly believe that the Forge is in need of such a system (I believe
Nigel brought up the proposal of metadata for this), and it should be
best practice to design modules to rely on this as much as possible.

BTW Nigel: Separating this thread from the "Cross-module (package)
dependencies" hasn''t really worked out, has it? ;)

Sincerely,
Felix

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

On 26/01/12 00:55, Trevor Vaughan wrote:
> I mentioned in a previous thread that I don''t see an issue with
having multiple identical resources compiled across the code base and
I''d like to add
> that to this thread since it''s related.
> 
> class a {
>   package { ''foo'': ensure =>
''present'' }
> }
> 
> class b {
>   package { ''foo'': ensure =>
''present'' }
> }
> 
> include ''a''
> include ''b''
> 
> Should work. However, if the two resources differ, this should be a compile
error. In a perfect world, you wouldn''t have this issue, but it
shouldn''t
> be an error since you''re applying identical code.
I think this is a small improvement, but it is still forcing strong coupling -
two parts of the manifest have to know about each other and agree on how things
are defined.  It means writing self-contained manifests will still be hard or
impossible, unless *everyone* adopts the (arbitrary) convention of defining only
"ensure => present" or somesuch.

For example, suppose in one place I need a file to exist, and in another I also
need it to be executable.  Oh dear, I can''t do that.

N

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

On 26/01/12 01:25, Ashley Penney wrote:
> All I know is that telling users "If you download 5 modules from
puppet forge
> make sure you go through them all, extract any duplicating resources into
> random modules that exist purely to allow you to realize packages"
instead leads
> to a really bad user experience.
+1 from me.

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

On 01/26/2012 11:21 AM, Nick wrote:
> For example, suppose in one place I need a file to exist, and in another I
also
> need it to be executable.  Oh dear, I can''t do that.
That, and you''d need to merge require/before etc. Such things
aren''t
trivial. Nan put it this way:

On 01/25/2012 03:59 PM, Nan Liu wrote:
>> Module a:
>> >
>> >  file { "/foo/bar":
>> >   ensure => ''present'',
>> >   owner => ''root'',
>> >   content => "blah blah",
>> >  }
>> >
>> >
>> > Module b:
>> >
>> >  file { "/foo/bar":
>> >   ensure => ''present'',
>> >   mode => ''0774'',
>> >  }
>> >
>> >
>> > Currently Puppet doesn''t allow them to co-exist.  It
would be nice
if instead it
>> > could be told to check these definitions are consistent, and then
enforce the
>> > union of the two.  The same principle could apply to users,
groups,
packages,
>> > and presumably any other resources.
> How would this be implemented in a sane way to deal with any
> attributes that are hash/array? Merge, merge+unique, fail? What if we
> add relationship (require/before) or other meta-parameters to the mix?
> If I use the puppet config_version feature to track what resource is
> changed by which line of puppet code for auditing purpose, how would I
> audit a single attribute that can be due to multiple line of code?
> Once I started thinking about define types (which behave like a
> resource), it''s gets rather complex especially with conditional
> branching in the define type.
>
> Don''t get me wrong, this clearly would be a useful feature, but
I''m
> interested only if the rules of how this would behave can be clearly
> expressed and understood, otherwise this will be a maze of pain trying
> to figure out what part of the code broke something.
-- 
You received this message because you are subscribed to the Google Groups
"Puppet Users" group.
To post to this group, send email to puppet-users@googlegroups.com.
To unsubscribe from this group, send email to
puppet-users+unsubscribe@googlegroups.com.
For more options, visit this group at
http://groups.google.com/group/puppet-users?hl=en.

On Jan 20, 4:34 pm, Cody <c.a.herri...@gmail.com>
wrote:
> On Jan 20, 6:39 am, jcbollinger <John.Bollin...@stJude.org> wrote:
>
>
>
> > On Jan 20, 3:00 am, Dan Bode <d...@puppetlabs.com> wrote:
>
> > > * the big reason I keep on leaning on it is for package
dependencies. Often
> > > something needs an additional package installed (and it is
possible that
> > > other modules may have that same package dependency, and I
don''t want to
> > > have to create a new class every time that I need another package
> > > (especially for something complicated that may have tons of
package
> > > dependencies)
>
> > You describe one of the core use cases for virtual resources.  Instead
> > of relying on the defined() function, you can
> > 1) define virtual Package resources in some central place(s) for all
> > the packages your nodes might want to manage,
> > 2) include that class wherever needed, and
> > 3) *realize* Packages as appropriate wherever you know you need
> > certain ones.
>
> > That avoids parse-order issues, doesn''t require you to keep
multiple
> > definitions of the same resource synchronized, and makes it easier to
> > find specific Package definitions among your manifests.  In some cases
> > you might even be able to leverage collections with selection
> > predicates to simplify and clarify your manifests.  I don''t
see a
> > single reason to prefer use of ''defined'' for this
case.
>
> Defining all somewhat common packages in a central location becomes
> unrealistic when you no longer "control" the code that is in
every
> module you use.  If you obtain five modules from the forge and they
> all require a specific package and so all define that package your not
> going to convince, nor is it a good design to require everyone to move
> the package definitions from that collection of modules.  They need to
> function as a collection out of the box.

On the contrary, if that''s what is required for modules to be
interoperable, and module authors indeed want their modules to be
interoperable, then I think it would be relatively easy to persuade
most authors to do it.  I can think of ways to make it work, and I''ll
write more on that in some other thread.

Sticking to the subject at hand, however, there remains the issue that
defined() does not adequately solve the shared resource problem
anyway.  If two modules both rely on the same resource then it is not
safe for one to assume that the definition of that resource provided
by the other meets all its needs.  The ''defined'' function can
maybe
ensure that the manifests compile, but it cannot ensure that the
resulting configuration is correct.  That makes it worse than useless
for the purpose, as far as I''m concerned.


John

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

On 01/26/2012 04:28 PM, jcbollinger wrote:
> The ''defined'' function can maybe
> ensure that the manifests compile, but it cannot ensure that the
> resulting configuration is correct.  That makes it worse than useless
> for the purpose, as far as I''m concerned.
Exceptionally good point.

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

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

(I know I''m late in responding; I''m a couple of months behind
in reading
the mailing-list, and still have 2000 unread.  I have read this entire
thread, though.)

On 2012-01-19 18:18, Nigel Kersten wrote:
> I''m looking for strong opinions on whether we should or
shouldn''t deprecate
> the defined() function for Telly, the next major Puppet release this year.
> 
> jcbollinger put it quite well in another thread:
> 
> "Use of the "defined" function introduces a parse-order
dependency, and the
> additional work you need to do to ensure that that dependency is always
> fulfilled overcomes any simplicity advantage that might otherwise
exist."
- -1 until there is some other, usable, way of realizing the good things
you can achieve with defined().

My use case for defined() is in definitions that wants to make sure
a package it needs is installed, and where that package varies between
instances of the definitions.  For example, I have a definition named
nrpe::check, which configures a Nagios NRPE check.  It wants to make
sure the appropriate nagios-plugins-PLUGIN is installed, so it does
(simplified, and just retaining the code dealing with the package):

    define nrpe::check($plugin='''',
$checker='''', $args=[], $prefixcmd=[])
    {
	$pluginpkg = $plugin ? {
	    ''''      => "nagios-plugins-${name}",
	    0       => '''',
	    default => $plugin,
	}

	if $pluginpkg != ''''  and  ! defined(Package[$pluginpkg]) {
	    package {
		$pluginpkg: ensure => installed,
	    }
	}
    }

A user would do things like:

    nrpe::check { ''load'': args => [
''-w20,10,8'', ''-c30,18,10'' ]; }

and the "nagios-plugins-load" package will automatically be installed.
This becomes interresting when you have:

    nrpe::check { ''test_foo'': args =>
[''foo''], plugin =>
''nagios-plugins-ourtest''; }

and

    nrpe::check { ''test_bar'': args =>
[''bar''], plugin =>
''nagios-plugins-ourtest''; }

both using the same plugin, but with different parameters, in different
parts of the manifest for the node.  The cincher here is that there are
several dozens of Nagios plugin packages available (EPEL has 63, many
organizations probably have several of their own, and so on).

It would be possible to instead have the nrpe::check definition do

    include "nagios_plugin_package::${plugin}"

and force the user to define a class for each plugin they use (and I
could of course "pre-seed" by writing 63 of those myself in my nrpe
module).  But it becomes much easier on the users of the nrpe module
if they don''t need to do so.

This use of defined() is safe, since there is really only a single
declaration of each Package[''nagios-plugins-PLUGIN''] resource.

I''m not fond of defined(), as it is easy to get wrong, but it is also
quite useful in some cases.  It would be a shame to deprecate a useful
feature before there is a workable way to achieve the same end results
in place.


	/Bellman
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/

iEYEARECAAYFAk90nj8ACgkQDGpP8Cv3aqL2WQCeMkEru8FitrIIzuG3HNdQGiPe
18oAnR94buhofKlbMBa/S+6zEtxX5jYa
=iTQw
-----END PGP SIGNATURE-----

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

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

(Coming late to the discussion, I know.  Sorry.)

On 2012-01-26 11:19, Felix Frank wrote:

[Regarding common modules for installing packages.]
> Thinking about other examples of similar systems (CPAN, Gems, Pear, you
> name it), module dependencies are commonplace (and usually coupled with
> a system that will automatically resolve them for the end user).
A big difference is that in those other systems, you don''t typically
see modules on the level of

    def add_17(n)
        return n + 17
    end

And of course, we need another module for doing

    def add_23(n)
        return n + 23
    end

Puppet modules for making sure a single package is installed is
on the same sofistication level as the above two.  It''s silly.
What would the reaction of the Python, Perl and Ruby communities
be if someone wanted modules like that in PyPI, CPAN and RubyGems?
Why should we want such sillinesses in the Puppet Forge?


	/Bellman
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/

iEYEARECAAYFAk90ocUACgkQDGpP8Cv3aqK7mgCdEQlPE70a1bJncrmeC2fvqv+V
9OoAn36vzrLoyCsB976MUCmOjeJo/IN5
=tbQM
-----END PGP SIGNATURE-----

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

On Mar 29, 12:39 pm, Thomas Bellman <bell...@nsc.liu.se>
wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
>
> (I know I''m late in responding; I''m a couple of months
behind in reading
> the mailing-list, and still have 2000 unread.  I have read this entire
> thread, though.)
>
> On 2012-01-19 18:18, Nigel Kersten wrote:
>
> > I''m looking for strong opinions on whether we should or
shouldn''t deprecate
> > the defined() function for Telly, the next major Puppet release this
year.
>
> > jcbollinger put it quite well in another thread:
>
> > "Use of the "defined" function introduces a parse-order
dependency, and the
> > additional work you need to do to ensure that that dependency is
always
> > fulfilled overcomes any simplicity advantage that might otherwise
exist."
>
> - -1 until there is some other, usable, way of realizing the good things
> you can achieve with defined().
>
> My use case for defined() is in definitions that wants to make sure
> a package it needs is installed, and where that package varies between
> instances of the definitions.  For example, I have a definition named
> nrpe::check, which configures a Nagios NRPE check.  It wants to make
> sure the appropriate nagios-plugins-PLUGIN is installed, so it does
> (simplified, and just retaining the code dealing with the package):
>
>     define nrpe::check($plugin='''',
$checker='''', $args=[], $prefixcmd=[])
>     {
>         $pluginpkg = $plugin ? {
>             ''''      =>
"nagios-plugins-${name}",
>             0       => '''',
>             default => $plugin,
>         }
>
>         if $pluginpkg != ''''  and  !
defined(Package[$pluginpkg]) {
>             package {
>                 $pluginpkg: ensure => installed,
>             }
>         }
>     }

Yes, that''s exactly the sort of thing you should avoid for reliability
and maintainability.  It will not do what you want if the declaration
of the desired package has not yet been parsed, and parse order is
tricky to predict except by means that offer better solutions anyway.

That particular example appears to be a good use case for virtual
resources:

class nrpe::plugins {
    @package { ''nagios-plugins-plugin1'': ensure =>
''installed'' }
    @package { ''plugin2'': ensure =>
''installed'' }
}

define nrpe::check($plugin='''', $checker='''',
$args=[], $prefixcmd=[])
{
    include ''nrpe::plugins''

    $pluginpkg = $plugin ? {
        ''''      => "nagios-plugins-${name}",
        0       => '''',
        default => $plugin,
    }

    if $pluginpkg != '''' {
        realize Package[$pluginpkg]
    }
}

> A user would do things like:
>
>     nrpe::check { ''load'': args => [
''-w20,10,8'', ''-c30,18,10'' ]; }
>
> and the "nagios-plugins-load" package will automatically be
installed.
> This becomes interresting when you have:
>
>     nrpe::check { ''test_foo'': args =>
[''foo''], plugin =>
''nagios-plugins-ourtest''; }
>
> and
>
>     nrpe::check { ''test_bar'': args =>
[''bar''], plugin =>
''nagios-plugins-ourtest''; }
>
> both using the same plugin, but with different parameters, in different
> parts of the manifest for the node.  The cincher here is that there are
> several dozens of Nagios plugin packages available (EPEL has 63, many
> organizations probably have several of their own, and so on).
>
> It would be possible to instead have the nrpe::check definition do
>
>     include "nagios_plugin_package::${plugin}"
>
> and force the user to define a class for each plugin they use (and I
> could of course "pre-seed" by writing 63 of those myself in my
nrpe
> module).  But it becomes much easier on the users of the nrpe module
> if they don''t need to do so.

And defining separate classes for the needed packages is indeed
another avenue to the desired result.

> This use of defined() is safe, since there is really only a single
> declaration of each Package[''nagios-plugins-PLUGIN'']
resource.

Your particular usage may work reliably in your context, but I
wouldn''t call it "safe".  It risks breakage if any of the
packages in
question indeed are declared elsewhere in your manifests, and it is
insufficient if ever you have a need for any of the plugin packages to
be declared with nonstandard properties (e.g. a specific version).

That usage of defined() requires faithful adherence to usage patterns
that Puppet has no means to check or validate, and there are viable
alternatives (even if you don''t like them as well for your specific
case).  That''s why that use of defined() is under consideration for
deprecation.

> I''m not fond of defined(), as it is easy to get wrong, but it is
also
> quite useful in some cases.  It would be a shame to deprecate a useful
> feature before there is a workable way to achieve the same end results
> in place.

The use you describe has much in common with the module
interoperability problem.  I don''t expect any fundamental advances in
that area in Telly, but I have high hopes for the following major
release.


John

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