Hi Ginnie,
See below.
On 07/21/10 17:56, Keith Mitchell wrote:
Hi Ginnie,
Responses below.
On 07/20/10 03:00 PM, Ginnie Wray wrote:
Hi Keith -
Thanks for the code review....see my response inline.
ginnie
On 07/16/10 02:18 PM, Keith Mitchell wrote:
On 07/16/10 12:46 PM, Ginnie Wray wrote:
Hi -
I've posted the python logging files for code review.
http://cr.opensolaris.org/~ginnie/install_logging_pyfiles/
I would like feedback by July 27th, COB.
Thanks,
ginnie
_______________________________________________
caiman-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/caiman-discuss
101-102: If the error is not 17, I imagine there will be a further
error raised by the __init__ call below, so printing the traceback
seems redundant. Need to either error out here, or simply pass
(regardless of the error code) and let the below __init__ call raise
exceptions if things aren't set-up right.
Here is my logic for setting this up. The FileHandler Class doesn't do
any checking for directory paths, and it is sporadic on creating them
and returns a traceback in some instances.
For example, if /var/tmp/install exists, and I want to create
/var/tmp/install/log/default_log, it will work because the log
directory doesn't exist. But, if I create
/var/tmp/install/default_log, it fails because I'm not creating a new
directory in that scenario.
I didn't think the install logger should fail because the directory
didn't exist, so I added the os.mkdirs command to check for that. It
returns a traceback if the directory already exists (that is error
17), and that seems like an error that can be ignored in this case.
Correct, but the layout of that except block has all OSErrors getting
ignored, and those with an error 17 get printed out. I think the
printing can be skipped. For non-17 errors, the exception should be
re-raised (or ignored, and just let FileHandler see if it can handle it.
In addition, it is better not to hard code the 17. Maybe use errno.EEXIST
instead.
Sue
_______________________________________________
caiman-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/caiman-discuss
