Skip to content

(maint) Make Puppet have hard dependency on Facter 2.0#787

Closed
jeffweiss wants to merge 2 commits intopuppetlabs:masterfrom
jeffweiss:maint/master/make_facter_2_hard_dep
Closed

(maint) Make Puppet have hard dependency on Facter 2.0#787
jeffweiss wants to merge 2 commits intopuppetlabs:masterfrom
jeffweiss:maint/master/make_facter_2_hard_dep

Conversation

@jeffweiss
Copy link
Copy Markdown
Contributor

Puppet 3.0 requires Facter 2.0. Make the dependency hard and have Puppet
fail with a useful error message if Facter doesn't exist or is the wrong
version.

lib/puppet.rb Outdated
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not a big fan of having an "exit" in the middle of a module definition...

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't blame you, this is within the Puppet module where we explicitly do want to halt execution because we have a hard dependency on Facter 2.0 which has not been met.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah but the module isn't driving execution... so it shouldn't be calling "exit". It should raise an exception and let whatever executable is driving the execution of the program decide whether to exit or not.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, yes. I pulled as-is from @kbarber 's structured facts branch. We can improve on how that's done.

Puppet 3.0 requires Facter 2.0. Make the dependency hard and have Puppet
fail with a useful error message if Facter doesn't exist or is the wrong
version.
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To do this "right" we should also remove the require 'facter' from the top, but this cascades NameErrors to a bunch of other places.

Rather than hard exiting from anything that happens to include Puppet
with an incorrect version of Facter, we'll instead raise a
Puppet::Error.
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This version seems much better to me. If we want to be really specific about error handling / messaging, we can catch this in either CommandLine.rb or bin/puppet.rb--those guys are responsible for execution. If this error class is too general to catch in those, we could make a more specific error class for it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants