This is essentially an internal error, but it *could* be a user error, if someone sets the owner of a file in puppet.conf, which is possible but rare.
Is it reasonable to just log the most common case (that it's an internal error), or should we try to find a different phrasing that allows for both? The rest of the code is easily +1, assuming it's been tested to provide reasonable output. On Sep 17, 2009, at 10:36 PM, Markus Roberts wrote: > > Reworks the error message to 1) make it clearer that it's an internal > error, not something the user did, 2) rearrange the sentence to make > it clearer that "setting" is being used as a noun 3) combined several > fields to increase the chance that the identifying information would > suffice to lead someone to the actual source of the error. > > Signed-off-by: Markus Roberts <[email protected]> > --- > lib/puppet/util/settings/file_setting.rb | 6 ++++-- > 1 files changed, 4 insertions(+), 2 deletions(-) > > diff --git a/lib/puppet/util/settings/file_setting.rb b/lib/puppet/ > util/settings/file_setting.rb > index 22e408a..573628f 100644 > --- a/lib/puppet/util/settings/file_setting.rb > +++ b/lib/puppet/util/settings/file_setting.rb > @@ -16,7 +16,8 @@ class Puppet::Util::Settings::FileSetting < > Puppet::Util::Settings::Setting > > def group=(value) > unless AllowedGroups.include?(value) > - raise SettingError, "Invalid group %s on setting %s. > Valid groups are %s." % [value, name, AllowedGroups.join(', ')] > + identifying_fields = > [desc,name,default].compact.join(': ') > + raise SettingError, "Internal error: The :group setting > for %s must be 'service', not '%s'" % [identifying_fields,value] > end > @group = value > end > @@ -28,7 +29,8 @@ class Puppet::Util::Settings::FileSetting < > Puppet::Util::Settings::Setting > > def owner=(value) > unless AllowedOwners.include?(value) > - raise SettingError, "Invalid owner %s on setting %s. > Valid owners are %s." % [value, name, AllowedOwners.join(', ')] > + identifying_fields = > [desc,name,default].compact.join(': ') > + raise SettingError, "Internal error: The :owner setting > for %s must be either 'root' or 'service', not '%s'" % > [identifying_fields,value] > end > @owner = value > end > -- > 1.6.4 > > > > -- I don't want the world, I just want your half. --------------------------------------------------------------------- Luke Kanies | http://reductivelabs.com | http://madstop.com --~--~---------~--~----~------------~-------~--~----~ You received this message because you are subscribed to the Google Groups "Puppet Developers" 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-dev?hl=en -~----------~----~----~----~------~----~------~--~---
