On Tue, Apr 12, 2011 at 2:36 PM, Jacob Helwig <ja...@puppetlabs.com> 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.
Note that this also solves the problem reported in: https://projects.puppetlabs.com/issues/7075 > > 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 > > -- > You received this message because you are subscribed to the Google Groups > "Puppet Developers" group. > To post to this group, send email to puppet-dev@googlegroups.com. > To unsubscribe from this group, send email to > puppet-dev+unsubscr...@googlegroups.com. > For more options, visit this group at > http://groups.google.com/group/puppet-dev?hl=en. > > -- Nigel Kersten Product, Puppet Labs @nigelkersten -- You received this message because you are subscribed to the Google Groups "Puppet Developers" group. To post to this group, send email to puppet-dev@googlegroups.com. To unsubscribe from this group, send email to puppet-dev+unsubscr...@googlegroups.com. For more options, visit this group at http://groups.google.com/group/puppet-dev?hl=en.
