Re: [Freeipa-devel] [PATCH] jderose 041 Fix logging

2010-02-09 Thread Jason Gerard DeRose
On Mon, 2010-02-08 at 11:38 -0500, Rob Crittenden wrote:
 Jason Gerard DeRose wrote:
  I lied one, more.
  
  Rob, I see you changed how the log level on the root logger is set in
  API.bootstrap()... unfortunately, under the server and CLI, the result
  is that the root logger always stays at its default level of
  logging.WARNING, so none of our info() nor debug() messages are going
  into the server log nor out to stderr (even with --debug).
  
  My solution is to unconditionally set the root logger to logging.DEBUG,
  the most verbose we use, and then configure the levels on individual
  handlers as appropriate (which we already do).
  
  Rob, I know you make this change because of problems with logging from
  the installer, so can you see if still works the way you want it to with
  this patch?  By the way, are you setting up your own logging handler in
  the installer, or using the ones configured in API.bootstrap()?
  
  Anyway, we really shouldn't release our alpha with broken logging.  Not
  nice to our brave and helpful testers.  ;)
 
 Jason, I think we can instead test for len(log.handlers) == 0 to 
 determine if we have already configured a file handler for it. Can you 
 confirm this? So if there are no handlers configured we set the log 
 level, otherwise we skip it.
 
 rob

Yep, that fixes it.  Updated patch attached (replaces my original 041
patch).
From d441e08c356f5003dafef409a9dc059b75bf4f3d Mon Sep 17 00:00:00 2001
From: Jason Gerard DeRose jder...@redhat.com
Date: Tue, 9 Feb 2010 04:57:23 -0700
Subject: [PATCH] Fix logging in CLI and server (take 2)

---
 ipalib/plugable.py |   15 ++-
 1 files changed, 10 insertions(+), 5 deletions(-)

diff --git a/ipalib/plugable.py b/ipalib/plugable.py
index 6b2c6f7..4473409 100644
--- a/ipalib/plugable.py
+++ b/ipalib/plugable.py
@@ -365,11 +365,16 @@ class API(DictProxy):
 self.env._finalize_core(**dict(DEFAULT_CONFIG))
 log = logging.getLogger()
 object.__setattr__(self, 'log', log)
-if log.level == logging.NOTSET:
-if self.env.debug:
-log.setLevel(logging.DEBUG)
-else:
-log.setLevel(logging.INFO)
+
+# If logging has already been configured somewhere else (like in the
+# installer), don't add handlers or change levels:
+if len(log.handlers)  0:
+return
+
+if self.env.debug:
+log.setLevel(logging.DEBUG)
+else:
+log.setLevel(logging.INFO)
 
 # Add stderr handler:
 stderr = logging.StreamHandler()
-- 
1.6.3.3

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Re: [Freeipa-devel] [PATCH] jderose 041 Fix logging

2010-02-09 Thread Rob Crittenden

Jason Gerard DeRose wrote:

On Mon, 2010-02-08 at 11:38 -0500, Rob Crittenden wrote:

Jason Gerard DeRose wrote:

I lied one, more.

Rob, I see you changed how the log level on the root logger is set in
API.bootstrap()... unfortunately, under the server and CLI, the result
is that the root logger always stays at its default level of
logging.WARNING, so none of our info() nor debug() messages are going
into the server log nor out to stderr (even with --debug).

My solution is to unconditionally set the root logger to logging.DEBUG,
the most verbose we use, and then configure the levels on individual
handlers as appropriate (which we already do).

Rob, I know you make this change because of problems with logging from
the installer, so can you see if still works the way you want it to with
this patch?  By the way, are you setting up your own logging handler in
the installer, or using the ones configured in API.bootstrap()?

Anyway, we really shouldn't release our alpha with broken logging.  Not
nice to our brave and helpful testers.  ;)
Jason, I think we can instead test for len(log.handlers) == 0 to 
determine if we have already configured a file handler for it. Can you 
confirm this? So if there are no handlers configured we set the log 
level, otherwise we skip it.


rob


Yep, that fixes it.  Updated patch attached (replaces my original 041
patch).



Works for me, ack, pushed to master

rob

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel


Re: [Freeipa-devel] [PATCH] jderose 041 Fix logging

2010-02-08 Thread Rob Crittenden

Jason Gerard DeRose wrote:

I lied one, more.

Rob, I see you changed how the log level on the root logger is set in
API.bootstrap()... unfortunately, under the server and CLI, the result
is that the root logger always stays at its default level of
logging.WARNING, so none of our info() nor debug() messages are going
into the server log nor out to stderr (even with --debug).


Hmm, I must've misread the logging code. I was sure it defaulted to NOTSET.



My solution is to unconditionally set the root logger to logging.DEBUG,
the most verbose we use, and then configure the levels on individual
handlers as appropriate (which we already do).

Rob, I know you make this change because of problems with logging from
the installer, so can you see if still works the way you want it to with
this patch?  By the way, are you setting up your own logging handler in
the installer, or using the ones configured in API.bootstrap()?


Yes, we set up a file logger. I wonder if there is a better way to 
detect if there is a root logger configured so we can avoid twiddling 
with the log level in api.bootstrap().




Anyway, we really shouldn't release our alpha with broken logging.  Not
nice to our brave and helpful testers.  ;)


Yup. I'll give it a test but I'm not sure forcing DEBUG is going to be 
the best way to deal with this.


rob

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel