(maint) Make loglevel default more efficient

[MartyEwings]

This line is not sane for the setting of the log_level default

loglevel = %x{ #{puppet_path} config --section agent --log_level notice print log_level }.chomp

this sets log_level to notice, and then return the output of the log_level setting

  loglevel = "notice"

is the same thing with fewer steps

[joshcooper]

The current code is written that way because log_level is a puppet setting and may be stored in puppet.conf. So in eb63fd6 we added logic to run puppet agent --configprint log_level to lookup the current level. Later the command was changed to puppet config print --section agent log_level, but if you set log_level=debug in puppet.conf, it generates a bunch of unneeded output. So in 1e58925 we explicitly specify --log_level notice for just this one command invocation. Kind of convoluted, but necessary since we don't have a simple way of looking up puppet config values without running puppet config

[MartyEwings]

Hi @joshcooper yeah I understand you need it to be notice level, but this one parameter command can never output anything other than the string "notice" no matter what in puppet.conf, I'm not understanding the need to do a one time puppet config set to notice, and print the fact notice was set to a parameter within that command?

[joshcooper]

@MartyEwings I think the desired outcome was if you have log_level=debug set in puppet.conf, then the call to puppet config print log_level --log_level notice should display the value that was stored in puppet.conf and not the overridden value on the CLI. Otherwise, like you say, what's the point.

So I think we either need to go with your PR and update the reference docs for log_level to say it doesn't affect the windows service (which is what we were trying to solve in PUP-3376).

Or we need to special case puppet config print log_level to ignore the value specified on the CLI and return the value specified in puppet.conf. It's possible to do that doing something like:

        value = Puppet.settings.setting(:log_level).default

        sources = Puppet.settings.configsearchpath(Puppet.settings[:environment], options[:section])
        sources.each do |src|
          next unless src
          next if src.name == :cli

          values = Puppet.settings.searchpath_values(src)
          next unless values

          v = values.lookup(:log_level)
          next unless v

          value = v
          break
        end

[joshcooper]

I think the PR is fine, it preserves the current behavior in fewer steps like you said. I'll file a separate issue that we've lost the ability for the windows service to observe the log_level setting.

[CLAassistant]

All committers have signed the CLA.

[joshcooper]

@MartyEwings could you rebase and resolve conflicts?