Re: [etch-users] Request: etchdebug.log to configurable destination

[Jason H. <jh...@ap...>]

(Moving this to etch-devel, bcc'ing out etch-users)

It looks like you're adopting a different syntax for your "etchdebuglog" from the accepted syntax for the existing config file entries.  E.g. the line for matching the "auth_enabled" option is:

if line =~ /^\s*auth_enabled\s*=\s*(.*)/

Whereas you have:

if line =~ /^etchdebuglog:/

I think we want to retain a common syntax.

For the later half of your changes I personally find ternary operator syntax hard to read at a glance.  My wife tells me that's because I'm not a real developer, so shrug, but I have to stare at this a while to figure out what is going on:

debug ? @dlogger.level = Logger::DEBUG : @dlogger.level = Logger::INFO

Whereas this is obvious to me at a glance:

if debug
  @dlogger.level = Logger::DEBUG
else
  @dlogger.level = Logger::INFO
end

Maybe I'm special.

Jason

On Aug 8, 2011, at 11:40 PM, Pat O'Brien wrote:

> sounds good - how does this look:
> 
> ---------------------------->%------------------------------
> wordup:etch pobrien$ svn diff
> Index: server.rb
> ===================================================================
> --- server.rb   (revision 298)
> +++ server.rb   (working copy)
> @@ -24,14 +24,16 @@
>      end
>      @@configbase
>    end
> -  
> +
>    @@auth_enabled = nil
>    @@auth_deny_new_clients = nil
> +  @@etchdebuglog = nil  
>    def self.read_config_file
>      config_file = File.join(configbase, 'etchserver.conf')
>      if File.exist?(config_file)
>        auth_enabled = false
>        auth_deny_new_clients = false
> +      etchdebuglog = nil
>        IO.foreach(config_file) do |line|
>          # Skip blank lines and comments
>          next if line =~ /^\s*$/
> @@ -47,7 +49,11 @@
>              auth_deny_new_clients = true
>            end
>          end
> +        if line =~ /^etchdebuglog:/
> +          etchdebuglog = line.split.pop
> +        end
>        end
> +      @@etchdebuglog = etchdebuglog
>        @@auth_enabled = auth_enabled
>        @@auth_deny_new_clients = auth_deny_new_clients
>      end
> @@ -203,12 +209,11 @@
>      @facts = facts
>      @tag = tag
>      @debug = debug
> -    @dlogger = Logger.new(File.join(Rails.configuration.root_path, 'log', 'etchdebug.log'))
> -    if debug
> -      @dlogger.level = Logger::DEBUG
> -    else
> -      @dlogger.level = Logger::INFO
> -    end
> +
> +    @@etchdebuglog ? @dlogger = Logger.new(@@etchdebuglog) :  @dlogger = Logger.new(File.join(Rails.configuration.root_path, 'log', 'etchdebug.log'))
> +
> +    debug ? @dlogger.level = Logger::DEBUG : @dlogger.level = Logger::INFO
> +
>      @fqdn = @facts['fqdn']
>  
>      if !@fqdn
> ---------------------------->%------------------------------
> 
> -pat
> 
> On Mon, Aug 8, 2011 at 5:54 PM, Jason Heiss <jh...@ap...> wrote:
> My thoughts:
> 
> - Don't duplicate the logic of the self.read_config_file method, add your option into the existing config file parsing in that method.
> 
> - Rather than an option for a log directory (your "log_dir"), I'd vote for an option that specifies the full path for this particular log file.  I.e. "debuglog=/path/to/something" or similar.  I think giving a log directory option is misleading, as most of the "etch server" logs are actually emitted by Rails and the Ruby app server (unicorn or the like) and as such would still end up in <rails root>/log even if the user specified a different log directory in etchserver.conf.  The etch debug log is the only log file emitted by the etch server code itself.
> 
> Jason
> 
> On Aug 7, 2011, at 11:45 PM, Pat O'Brien wrote:
> 
>> Well, I worked out a fix for this, although I am unsure if it's a good fix or not.
>> 
>> First, here is the diff:
>> 
>> ------------------------------>%------------------------------
>> wordup:server pobrien$ svn diff
>> Index: lib/etch/server.rb
>> ===================================================================
>> --- lib/etch/server.rb  (revision 298)
>> +++ lib/etch/server.rb  (working copy)
>> @@ -203,12 +203,24 @@
>>      @facts = facts
>>      @tag = tag
>>      @debug = debug
>> -    @dlogger = Logger.new(File.join(Rails.configuration.root_path, 'log', 'etchdebug.log'))
>> -    if debug
>> -      @dlogger.level = Logger::DEBUG
>> -    else
>> -      @dlogger.level = Logger::INFO
>> +    @log_dir = nil
>> +    config_file = File.join(Etch::Server.configbase, 'etchserver.conf')
>> +    if File.exist?(config_file)
>> +      IO.foreach(config_file) do |line|
>> +        # Skip blank lines and comments
>> +         next if line =~ /^\s*$/
>> +         next if line =~ /^\s*#/
>> +        line.chomp
>> +        if line =~ /^log_dir:/
>> +          @log_dir = line.split.pop
>> +        end
>> +      end
>>      end
>> +
>> +    @log_dir ? @dlogger = Logger.new(File.join(@log_dir, 'etchdebug.log')) :  @dlogger = Logger.new(File.join(Rails.configuration.root_path, 'log', 'etchdebug.log'))
>> +
>> +    debug ? @dlogger.level = Logger::DEBUG : @dlogger.level = Logger::INFO
>> +
>>      @fqdn = @facts['fqdn']
>>  
>>      if !@fqdn
>> ------------------------------>%------------------------------
>> 
>> Here are my problems with the change: the first is that I'm not sure if this config should go into <etchbase>/etchserver.conf or if it should be within the RAILS_ROOT/config/ directory. This feels like something that should be configured along with the rails app rather than in /etc/etchserver, but from what I understand (and I am in no way, shape or form versed in rails) in rails 2.x.x there is no generic config file to handle something like this. There is a railscast (http://railscasts.com/episodes/85-yaml-configuration-file) that touches upon this, and they basically recommend RAILS_ROOT/config/config.yml along with some other necessary changes to actually have the config file loaded.
>> 
>> Second, it looks like the <etchbase>/etchserver.conf file is currently only used to define two things:  @@auth_enabled and @@auth_deny_new_clients = nil, and the way these are loaded is by parsing the config file the same way I parse it for the log_dir setting. I would prefer etchserver.conf file to be a YAML file and we could load it that way, but I haven't looked too deeply as to what else this file is used for so I don't really want to make any recommendations on how to change it.
>> 
>> Third, I haven't looked hard enough to see whether or not anything else references a log directory of any kind and because of my two issues above I won't bother until there's a bit more discussion on it.
>> 
>> Other than that the change works fine and behaves as you would expect it to, your etchserver.conf file would look like this:
>> 
>> log_dir: /whatever/directory/your/heart/desires
>> 
>> -pat
>> 
>> 
>> 
>> 
>> On Sun, Aug 7, 2011 at 10:19 PM, Pat O'Brien <obr...@gm...> wrote:
>> I've never bothered to try this so I don't know if it's configurable or not. If it's not open a bug ticket and I'll take a look at it when I get some spare time this week.
>> 
>> Just as a side note: my personal preference is to not have system logs end up in /var/log, but to each their own :)
>> 
>> -pat
>> 
>> 
>> 
>> On Wed, Aug 3, 2011 at 3:27 PM, Johnston, Nathaniel <Nat...@ca...> wrote:
>> Etch Mensches,
>> 
>> I am not sure how best to do this, but I am trying to move my Etch logs over to /var along with the other logfiles I have.  I noticed that on line 205 of lib/etchserver.rb the etchdebug.log file is specified as:
>> 
>> @dlogger = Logger.new(File.join(Rails.configuration.root_path, 'log', 'etchdebug.log'))
>> 
>> Is there a way that this could leverage the value of config.log_path/paths.log or another configurable parameter so that we could override the location of this file?
>> 
>> --N.
>> 
>> 
>> 
>> ------------------------------------------------------------------------------
>> BlackBerry&reg; DevCon Americas, Oct. 18-20, San Francisco, CA
>> The must-attend event for mobile developers. Connect with experts.
>> Get tools for creating Super Apps. See the latest technologies.
>> Sessions, hands-on labs, demos & much more. Register early & save!
>> http://p.sf.net/sfu/rim-blackberry-1
>> _______________________________________________
>> etch-users mailing list
>> etc...@li...
>> https://lists.sourceforge.net/lists/listinfo/etch-users
>> 
>> 
>> ------------------------------------------------------------------------------
>> BlackBerry&reg; DevCon Americas, Oct. 18-20, San Francisco, CA
>> The must-attend event for mobile developers. Connect with experts. 
>> Get tools for creating Super Apps. See the latest technologies.
>> Sessions, hands-on labs, demos & much more. Register early & save!
>> http://p.sf.net/sfu/rim-blackberry-1_______________________________________________
>> etch-users mailing list
>> etc...@li...
>> https://lists.sourceforge.net/lists/listinfo/etch-users
> 
>