I should have changed the subject to RFC. This patch hasn't actually been merged into Facter.
-- Jacob Helwig On Tue, 12 Apr 2011 14:36:43 -0700, Jacob Helwig wrote: > > Instead of always grabbing the full set of facts and only grabbing > those we're interested in, we attempt to look up every fact by the > "normal" means of "fact name == containing file name", and only fall > back to loading all of the facts if this fails to find any of the > facts we're looking for. > > Paired-with: Max Martin <m...@puppetlabs.com> > Signed-off-by: Jacob Helwig <ja...@puppetlabs.com> > --- > > Local-branch: tickets/master/7039-multiple-facts-same-file > > Here's the proposed patch to potentially ease the overhead when asking > for specific facts. The concern I have is that I didn't see any > caching of fact values, which would mean potentially re-evaluating the > facts that we'd already found in order to get a few that were missed > because of mis-matches between the fact name, and the file name that > contains it. > > It seems like the options are: > * Always evaluate all facts once. > > * Potentially evaluate some facts twice, if we were unable to find > all facts by name. > > I'm reluctant to add caching of fact values into Facter, given where I > suspect this would need to be added, and that Puppet uses it as a > library on the agent side (this seems like it would require changes on > the agent side for cache invalidation). > > lib/facter/application.rb | 16 +++++++++++----- > 1 files changed, 11 insertions(+), 5 deletions(-) > > diff --git a/lib/facter/application.rb b/lib/facter/application.rb > index bd68149..9b0ba03 100644 > --- a/lib/facter/application.rb > +++ b/lib/facter/application.rb > @@ -10,12 +10,8 @@ module Facter > names = argv > > # Create the facts hash that is printed to standard out. > - # Pre-load all of the facts, since we can have multiple facts > - # per file, and since we can't know ahead of time which file a > - # fact will be in, we'll need to load every file. > - facts = Facter.to_hash > + facts = {} > unless names.empty? > - facts = {} > names.each do |name| > begin > facts[name] = Facter.value(name) > @@ -26,6 +22,16 @@ module Facter > end > end > > + # If names is empty, then we want all of the facts. If all of > + # the asked for names aren't in the facts hash, then it's likely > + # that we were asked for a fact that doesn't match the filename > + # it's stored in, so the autoloader couldn't find it. In this > + # case we just need to load all of the facts, since we have no > + # way of figuring out which file it might be in. > + if names.empty? || !names.all? { |n| facts[n] } > + facts = Facter.to_hash > + end > + > # Print the facts as YAML and exit > if options[:yaml] > require 'yaml' > -- > 1.7.4.3 >
signature.asc
Description: Digital signature