On 10/15/10 10:57 AM, Karen Tung wrote:
Hi Ginnie,
Please see my comments inline.
On 10/14/10 04:46 PM, Ginnie Wray wrote:
Hi -
Three things are playing into what's going on...
1. The logger init
2. Tests run with vs. without the engine.
3. The reset_engine function in engine_test_utils.py (not really part
of the current problem but part of the solution)
The logger setting up a FileHandler presents a problem in that
every time getLogger is called, the __init__ of InstallLogger is
executed. That's why Dermot was seeing output twice.
The problem I found with moving the FileHandler code as Karen and I
suggested is that everytime getLogger is called with a new child
logger, a FileHandler is added to the child. This results in
propagating a FileHandler to every child created. When the child logs
a message, the log message first logs using the child's FileHandler
and then checks for a parent and logs to the parent handler, even
though it's the same file. Thus the double output .(Engine logger +
checkpoint logger)
The problem with checking for the existence of the default log
prevents a FileHandler from being created, resulting in missing output.
Dermot's non-engine code tests worked before because he was
generating his own LOGGER instead of using self.logger. Now that he
is using self.logger, when the tests that run w/o the engine are
executed, they won't have a handler associated with them, since the
InstallLogger sets up the default file handler.
My suggestion for how to solve this involves a couple of things.
1. For tests that are run without the engine, they should set up the
InstallLogger class in the setUp function of the unit tests. That way
the functionality of the InstallLogger is available, even w/o the
engine.
So the non-engine test code would need to add:
from solaris_install.logger import InstallLogger
def setUp(self):
'''DEBUG'''
logging.setLoggerClass(InstallLogger)
maybe a teardown as well. That doesn't seem to be a problem though,
because the python logger does checking for this.
2. I would like to change the logger.py code to check to see if the
InstallLogger.ENGINE param is set. If not, execute the default
FileHandler creation. If it is, skip it. This solves the double
output issue. Instead of the check to see if the log path exists,
I'll add a check for the following:
if self.ENGINE is None:
add filehandler code
While this approach works to solve this problem, it makes the
correctness of
InstallLogger dependent on the engine.
If InstallLogger is used outside of the engine context, we would get
the double output
problem Dermot reported when child logger is used again, because
self.ENGINE will
always be None, and file handlers will get added when child loggers
are instantiated.
To guarantee the correctness of the InstallLogger's behavior, I think
it is best to
somehow detect whether we are adding a logger that's just a child of
an existing
logger. If so, don't add the default file handler.
An alternative approach, is to save the filehandler value that gets
passed to
logging.Logger.addHandler() in a variable variable. To make a decision
for adding the default file handler or not in InstallLogger.__init__(),
we check that variable. In addition, in the reset_engine() function,
we check to see whether the variable containing the default file handler
is None, if not, the reset_engine() function will call
logging.Logger.removeHandler(),
and then, set the value to None for the next test. I have not try
this in the
code, it should work, I think. :-)
Thanks,
--Karen
I prefer the latter approach - store the default FileHandler in a global
variable in the install logging module, and only add it if it's not None.
The reset_engine() code would only have to reset that variable to None
(since the handler is associated with a logger instance, and the logger
instances are reset, it wouldn't need to explicitly remove the handler
from the logger).
3. In the engine_test_utils.py, reset_engine function, we should also
set the InstallLogger.ENGINE variable back to None. That will enable
item 2 to work properly.
from solaris_install.logger import InstallLogger
InstallLogger.ENGINE = None
I made these changes and ran Dermot's tests, and I think it works. I
have a defaultlog on Indiana-Build in
/var/tmp/install/default_log.26802. Dermot, do you want to take a
look and see if it's complete?
thanks,
ginnie
_______________________________________________
caiman-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/caiman-discuss
