(#17458) Undo minor performance change in 2.7.x#1271
Merged
pcarlisle merged 3 commits intopuppetlabs:2.7.xfrom Nov 8, 2012
Merged
(#17458) Undo minor performance change in 2.7.x#1271pcarlisle merged 3 commits intopuppetlabs:2.7.xfrom
pcarlisle merged 3 commits intopuppetlabs:2.7.xfrom
Conversation
Commit b5ed15b attempted to require 'puppet' at the right time, but failed. The command_line required puppet before the application was required and its run_mode determined. As a result, `puppet master` was using the default :user run_mode, causing its pidfile to be named 'agent.pid' among other things. This comit reverts b5ed15b.
In commit, 20efe94, a change was made to Puppet::Util.absolute_path? to require the Puppet module only if necessary. This was done as an optimization since re-requiring a file is much slower than checking if a constant is defined. However, defined?(Puppet) is always true when called within the Puppet::Util.absolute_path? method, so effectively, absolute_path? would never require the Puppet module, when previously it used to, in certain exceptional flows. In the normal flow, it wasn't an issue, because when puppet runs an internal subcommand (Application), the app requires the Puppet module, which loads features, which creates an autoloader, which calls the absolute_path? method. At this point, requiring puppet is not needed. However, if an external subcommand was specified, e.g. `puppet foo`, where `puppet-foo` is an external executable, or if no subcommand was specified, e.g. `puppet`, the CommandLine would attempt to resolve the external subcommand using Puppet::Util.which, which calls absolute_path? at a time when the Puppet module and features haven't been loaded, causing #17458. Ideally, the exceptional cases should be requiring puppet rather than relying on the absolute_path? method to do so. However, in 3.x, this entire logic has been reworked. In 3.x, the default confdir/vardir locations are solely based on whether we're running as root or not, and not the application's run_mode setting. As a result, we can and do require puppet early on. For 2.7.x, the safest thing is to revert the change in the require 'puppet' behavior.
Commit 20efe94 broke running `puppet` and `puppet-*` external subcommands. This adds an acceptance test to make sure puppet doesn't blow up when given an unknown external subcommand. Paired-with: Josh Cooper <josh@puppetlabs.com>
pcarlisle
added a commit
that referenced
this pull request
Nov 8, 2012
…ormance-fix (#17458) Undo minor performance change in 2.7.x
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.