Issue #6352 has been updated by Daniel Pittman. Status changed from In Topic Branch Pending Review to Needs More Information
My key concern with this change is that it doesn't really seem to achieve the desired results. It allows Facter to be mechanically pluggable, but there are still piles of ties binding it tightly to the rest of the product. For example, there are still mechanical dependencies `puppet/defaults.rb`, the catalog compiler, and a whole pile of providers that consume data from Facter by fact name and the structure of the value. With the provider confines, that is an unknowable set of dependencies on the data. It doesn't seem realistic to me that we can actually separate the two without substantial effort to break the other dependency - and, more over, it doesn't seem like that is a good investment of effort. Is the value in this solely that we can more easily substitute a "Facter lite" into place, for testing, or that doesn't have the same set of bugs that Facter does? Those seem to have some value, albeit limited, but I am not certain the additional extension point is really worth the cost of maintaining it for a single user, plus our test code. That said, I am happy to be convinced of this: if there is value somewhere that I don't see, I will merge this change in. At least, on the basis that we can then back out if trying to use it doesn't show value - we shouldn't refuse to experiment. Outside of that, it isn't clear what the semantics of the `load` method on Facts are. That doesn't need anything but inline documentation, together with notes about the relationship of that one method on the specific indirection, and the rest of the indirection infrastructure, but it should have those. Without them everyone else is left guessing when that should be called, or why, and more importantly, anyone who takes advantage of the new terminus to be able to replace Facter has to guess what they should do in implementing the `load` method - or what it should mean when it gets called. (eg: I /think/ it flushes any cached data, and collects it again, but I am not certain without going and reading what Facter does in response to the `clear` call.) Finally, in the area of caching you mention that you considered having the indirection fetch a single fact value, rather than cache, but that the "fetch all, serve from the cache" approach seemed like the lesser evil. Assuming we go forward, can you expand further on that - it seems to me like fetching individual facts is the best approach, because it allows encapsulation of all the behaviour (caching, lazy loading, etc, etc) in the terminus, and makes it easier to experiment with different policies down there? ---------------------------------------- Bug #6352: Puppet should only use Facter through a plugin interface https://projects.puppetlabs.com/issues/6352 Author: Luke Kanies Status: Needs More Information Priority: Normal Assignee: Daniel Pittman Category: plumbing Target version: Affected Puppet version: Keywords: facter dependencies Branch: luke/ticket/master/6352-facter_should_not_be_hard_coded Significant parts of Puppet (especially the client-side) use Facter directly. E.g., all of the provider subsystem is directly calling out to Facter. This causes a few problems: * Facter is hard-coded as a dependency for Puppet * Facter is required even if none of its data is used * Facter tends to be called more often than is necessary, because the data could just be cached in memory This should be fixed so that Puppet provides its own internal interfaces for interacting with Facter data, such that there's only one part of the system that actually talks to Facter. -- You have received this notification because you have either subscribed to it, or are involved in it. To change your notification preferences, please click here: http://projects.puppetlabs.com/my/account -- You received this message because you are subscribed to the Google Groups "Puppet Bugs" group. To post to this group, send email to [email protected]. To unsubscribe from this group, send email to [email protected]. For more options, visit this group at http://groups.google.com/group/puppet-bugs?hl=en.
