On 10/18/10 09:57 AM, Ginnie Wray wrote:
Hi Karen, Keith -
I've modified the logger and added a DEFAULTFILEHANDLER
variable and ran some tests on it. I didn't make any modifications
to the reset_engine utility. I thought the engine team would
want to take care of that.
Let me know if you need anything else from me.
ginnie
Hi Ginnie,
I made changes to the reset_engine() function in the engine's tests.
Ran the engine tests and made sure all is working as I expect.
I also ran the manifest tests and see that things look OK there.
I think your changes works well.
Thanks,
--Karen
On 10/15/10 14:05, Ginnie Wray wrote:
Hi Keith and Karen -
I think Karen's suggestion sounds good.
thanks for your feedback.
ginnie
On 10/15/10 12:19, Keith Mitchell wrote:
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
_______________________________________________
caiman-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/caiman-discuss
