[Re-send, first attempt failed...]

On 02/22/11 09:52 AM, Keith Mitchell wrote:
Hi Martin,

I'm still not sure adding the source parameter is the correct route. First, source should be a keyword (optional) argument here, if anything. But more importantly, consider the side effects of passing in source:

A FileHandler, fh, has an open reference to file X
Someone calls fh.transfer_log(source=Y, destination=Z)
The transfer_log code will now:
- close the reference to X, meaning that the filehandler is now no longer able to log to the place it's supposed to be logging to
- Copy Y to Z
- Continue logging by appending to Z.

This will lead to half of that FileHandler's log data in X, and half appended to Z. There's a major disconnect there that will be *very* confusing.

- Keith

On 02/21/11 05:27 PM, Martin Widjaja wrote:
Hi all,

Can I get a review of the logger bugs that are reported in the following 2 issues:
7006785  <http://bugs.opensolaris.org/bugdatabase/view_bug.do?bug_id=7006785>  
some of the logging unittests are broken
7012566  <http://bugs.opensolaris.org/bugdatabase/view_bug.do?bug_id=7012566>  
InstallLogger.close() is broken
The webrev is available at:
http://cr.opensolaris.org/~widjaja/logger-fix/ <http://cr.opensolaris.org/%7Ewidjaja/logger-fix/>

This is a continuation of the fix I did for 7012566. As I was unit testing my fixes, I ran into more issues causing several unit test errors, so decided to do all the fix in one txn.

Note that I attempted to keep all the old test functionalities such as transfer_log test with source specified, although some of you mentioned to me this is not really needed any longer. I thought that providing more flexibility to transfer_log (to specify source) is a better fix than removing the test case for it. Plus, not knowing the background story it seems to make more sense for me to keep all these tests around for now, at least.

Thanks,
Martin


_______________________________________________
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

Reply via email to