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.