On Sep 19, 2013, at 11:35 AM, John Bollinger <[email protected]> wrote:
> > > On Tuesday, September 17, 2013 1:36:34 PM UTC-5, Luke Kanies wrote: > On Sep 12, 2013, at 1:15 PM, John Bollinger <[email protected]> wrote: > >> >> >> On Wednesday, September 11, 2013 3:14:41 PM UTC-5, Luke Kanies wrote: >> On Sep 11, 2013, at 12:32 PM, John Bollinger <[email protected]> wrote: >> Even if we consider only ENC- and node-declared classes, another class being >> "in service to" one of those does not necessarily imply any essential >> requirement for containment. For example, suppose I have a 'site' module, >> declared at node level for all nodes, with this main class: >> >> class site { >> include 'site::motd' >> include 'site::users' >> } >> >> Suppose further that 'site::motd' manages only nodes' /etc/motd file, >> whereas 'site::users' manages login users. It is entirely reasonable that >> neither of those has any ordering requirements relative to anything else >> under management, so why should it be a best practice for class 'site' to >> contain them? > > Why wouldn't it be? I would certainly be surprised if I were looking at the > graph of classes and didn't see the site classes contained by the, um, site > class. > > > > Aren't you merely assuming the conclusion? I mean, you know containment is > not automatic, so do you have a reason for surprise that doesn't ultimately > boil down to the best practices assertion you are trying to support? I'm relying primarily on assertion, rather than data, it's true. However, 'include' used to always contain, and we only had problems in rare cases. Thus, I'm confident in assessing that it's generally safe to contain far more often than we do. > There are objective reasons why the site::* classes should not be contained > by class 'site'. Containment is not a valid end in itself; only inasmuch as > it supports acceptable resource application ordering is it useful. Where it > does not support that end it incrementally increases Puppet's run time and > resource consumption, and it creates a risk of needless dependency cycles. > > In the example, there is no need to constrain the order in which the site::* > classes are applied relative to anything else, thus there is no need for them > to be contained by class 'site' or any other class. It is therefore a > reasonable choice for class site to avoid the costs of containment, and any > best practices position that arbitrarily rejects that choice is highly > suspect. I disagree a bit. Without more containment than we have today, it's tough to know if our users have successfully provided as much dependency information as they should. It's true that containment by itself provides little value and has a cost, but I think we realistically can get to the point where everything that isn't contained deserves a warning, and thus provide much better information to our users about how they can improve their configurations. >> Certainly, DSL authors need to pay careful attention to relationships, >> probably moreso than most actually do. As such, I would be completely >> comfortable with a recommendation that authors default to choosing >> containment -- even one backed up by a warning, provided that it can be >> disabled by users who know better in particular cases. I am not persuaded, >> however, that unconditionally avoiding floating classes should be considered >> a best practice. >> >> A manifest set that omits needed relationships loses robustness, but a >> manifest set that includes unneeded relationships loses resiliency and is >> more costly. The best manifest set declares all needed relationships, but >> no more. > > I'm not really committed to this; it just seemed like a way to encourage the > right behavior without changing the functionality of the system. > > Do you have any ideas for how we might do that? > > > > We'll have to agree on what "the right behavior" is before I can suggest ways > to encourage it. I think warnings for some uncontained classes would serve > at least to help people detect unintentional containment failures, however, > and that is a good enough reason for me to endorse implementing them as an > optional feature. It is important to me that they remain optional > indefinitely, however. The right behavior isn't about containment; it's about dependencies. Have users sufficiently specified dependencies that their configurations are likely to succeed? If not, how can we tell? > Basically, I think there are very few cases where it actually makes sense to > have top-level classes; few enough that it's worth telling the user about > them because they probably want to fix it. > > > > I think it is highly desirable to have top-level classes when it does not > preclude representing all needed resource ordering requirements. The fewer > constraints must be honored during catalog application, the lighter Puppet > can be and the more flexibility it has. That includes the flexibility to > optimize -- for instance, by allowing resources for which batches are > supported to be grouped into larger batches, or perhaps by applying resources > in parallel. > > A warning such as the one proposed is fine, as long as people can silence it > in the event that they determine they actually don't want to fix the issue. I'm not opposed, just not sure how necessary it is. > If I'm right, then the choices are: > > * Rather than risk warning people about things that aren't actually problems, > let users continue to struggle with ordering > > * Risk erroneous warnings and people sometimes having to add irrelevant > containments in order for the vast majority of users to have a better > experience > > If I'm wrong, of course, then the whole thing is pointless. But that's the > trade off I'm thinking. > > > > So what's wrong with defaulting to the latter, and allowing users to opt for > the former? The ability to silence unwanted warnings is a highly desirable > feature, especially in shops where warnings are considered errors. Sure. I'd even go so far as to say we should maybe only add the warnings in a strict mode, or have a special mode that throws more warnings. -- Luke Kanies | http://about.me/lak | http://puppetlabs.com/ | +1-615-594-8199 -- 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 [email protected]. To post to this group, send email to [email protected]. Visit this group at http://groups.google.com/group/puppet-dev. For more options, visit https://groups.google.com/groups/opt_out.
