On Tue, Sep 3, 2013 at 12:44 PM, Luke Kanies <l...@puppetlabs.com> wrote:

> +1 to a function expressing containment but not declaring the class (#3).
>  It's the only option of the 4 provided that doesn't have the side-effect
> of adding the class to the catalog.  I've run across situations where a
> module may want to contain a third party class and relies on some other
> module to declare the class.  "site" modules are a common example.
>
>
> Sorry, why?  What's the benefit of requiring a second call?  Or, what's
> the downside of defaulting to 'include' for classes that have not yet been
> evaluated?
>

The concern of adding a class to the catalog is distinctly separate from
the concern of containing a class.  Smashing the two concerns together
makes it difficult to address all of the concerns.  The benefit of
requiring a second call is that the concerns are much easier to address
directly without edge cases and qualifiers for class parameters.

I mentioned a site module.  Consider this example.  What is the result?

  1 class site::motd($template = 'site/motd') {
  2   notify { 'motd': message => "template is $template" }
  3 }
  4
  5 class goal {
  6   notify { "site::motd::template":
  7     message => "::site::motd::template is ${::site::motd::template}
(should be ${::node_template})"
  8   }
  9 }
 10
 11 class container {
 12   contain('site::motd')
 13 }
 14
 15 node default {
 16   $node_template = 'site/motd.jeff'
 17   class { 'container': }
 18   -> class { 'goal':}
 19
 20   class { 'site::motd': template => $node_template }
 21 }

The result I expect is that the class declaration on line 20 trumps all
other includes because it is the most specific regarding explicit class
parameters.

The behavior we actually get with Puppet right now surprises me because I'm
quite confident that we decided include was fine to use along with resource
style class declarations so long as there is only one resource style
declaration...  However, what we actually get is this:

(3.3.0-rc2-16-gcfe704e)
$ puppet apply -v contain.pp
Error: Duplicate declaration: Class[Site::Motd] is already declared; cannot
redeclare at /workspace/serious/src/puppet/contain.pp:20 on node
agent1.example.org
Error: Duplicate declaration: Class[Site::Motd] is already declared; cannot
redeclare at /workspace/serious/src/puppet/contain.pp:20 on node
agent1.example.org

This is a bug in my opinion.  The catalog should compile and apply and we
should see "::site::motd::template is site/motd.jeff (should be
site/motd.jeff)" printed out.

In any event, it's important that we allow one class to contain another
class and defer explicit class parameters to other places.  If contains has
the side effect of declaring the class then this isn't possible unless the
end user carefully manages the parse order of their manifests.

> A separate, focused, function also keeps our support matrix small and more
> maintainable.  We'll want to implement the behavior consistently across all
> the ways of adding classes to catalogs if we implement the behavior for one
> of them.  These behaviors are burdensome to maintain over time.  A
> contains() function allows the behavior to work with all the ways of adding
> classes to the catalog while allowing us to maintain the code in one
> supported manner.
>
>
> Can you elaborate?  I don't know what you mean.  What's burdensome about
> having 'contain' call 'include'?
>

If contain calls include, how can you be sure some other module doesn't
declare the class before you when you want to provide explicit class
parameters yourself?  You can't be sure unless you take on the burden of
carefully managing the parse order of the manifests to make absolutely sure
your resource style declaration happens before include, contains, require,
or whatever new way we invent to add classes to the catalog in an exclusive
way.


> Do you think it's a mistake for 'require' to call 'include'?
>

I've never understood why require() exists in the language and I've never
used the function in any of the manifests or modules I've written.

Yes, it's a mistake for require to call include.  If the purpose of require
is to require another class and it has the side effect of including the
class in a manner that prevents me from later specifying explicit class
parameters, then yes it's a mistake to smash these concerns together.

require should require, include should include and contain should contain.
 Each function doing more or doing less than these clear and direct things
is a mistake.

-Jeff

-- 
You received this message because you are subscribed to the Google Groups 
"Puppet Developers" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to puppet-dev+unsubscr...@googlegroups.com.
To post to this group, send email to puppet-dev@googlegroups.com.
Visit this group at http://groups.google.com/group/puppet-dev.
For more options, visit https://groups.google.com/groups/opt_out.

Reply via email to