Menu
▾
▴
Re: [etch-devel] [etch-users] Request: etchdebug.log to configurable destination
|
From: Pat O'B. <obr...@gm...> - 2011-08-10 05:05:21
|
ok, changes made and ready for review:
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 =~ /^\s*etchdebuglog\s*=\s*(.*)/
+ etchdebuglog = $1
+ end
end
+ @@etchdebuglog = etchdebuglog
@@auth_enabled = auth_enabled
@@auth_deny_new_clients = auth_deny_new_clients
end
@@ -203,12 +209,19 @@
@facts = facts
@tag = tag
@debug = debug
- @dlogger = Logger.new(File.join(Rails.configuration.root_path, 'log',
'etchdebug.log'))
+
+ if @@etchdebuglog
+ @dlogger = Logger.new(@@etchdebuglog)
+ else
+ @dlogger = Logger.new(File.join(Rails.configuration.root_path, 'log',
'etchdebug.log'))
+ end
+
if debug
@dlogger.level = Logger::DEBUG
else
@dlogger.level = Logger::INFO
end
+
@fqdn = @facts['fqdn']
if !@fqdn
in regards to the ternary operators, I am pretty indifferent and more about
keeping the coding style in tact within a project so that's all fine with
me.
-pat
On Tue, Aug 9, 2011 at 5:01 PM, Jason Heiss <jh...@ap...> wrote:
> (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® 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® 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
>>
>>
>>
>
>
|
