Here's what I am proposing:

- FileHandler.transfer_log to have source as keyword (optional) arg to provide 
flexibility for people to use this
- FileHandler.transfer_log to throw an exception if it currently has an open 
reference to a file different from what's passed into the source

This is still combining the two functionalities somewhat into FileHandler but 
with explicit error handling mechanism to prevent people from doing whatever 
they can, but at the same time allowing ICT functionality which is already 
using FileHandler.transfer_log to continue to work until a later time we 
decided to move this somewhere.

I am not familiar with the transfer module, so if anyone (Keith and Ginnie in 
particular?) thinks this can be better off somewhere and we can shut source 
argument from filehandler forever, then we should discuss further.

Ginnie - Is the usage of this transfer_log pretty widespread in the ICT area?

Thanks,
Martin

On 2/22/2011 11:28 AM, Keith Mitchell wrote:
If that is the functionality needed, then what really is needed is two separate 
functions:

1) To transfer an arbitrary, *closed* and completed log file from one location 
to another. A glorified shutil.copy, as it were.
2) A function on the FileHandler that closes its current log file, moves it, 
and re-opens it for appending, to allow for continued logging to that file.

Combining the two into a single function that tries to do both is only going to 
lead to odd bugs and inconsistent log files.

#2 is how FileHandler.transfer_log() is currently implemented. Its behavior 
can't be changed without breaking DC's use case
#1 sounds an awful lot like what the transfer module does...

There's certainly some commonality - a helper function that does the "make dirs as 
needed, then move the file" portion might be warranted.

- Keith

On 02/22/11 10:28 AM, Virginia Wray wrote:
Hi -

We do need a means of transferring a source file to a destination. The icts 
have that functionality currently, and I've had a conversation with Darren and 
Matt around the need for moving various files to a different destination as 
part of the AI work.

Rather than making the transfer_logs funtionality tied only to the 
DEFAULTFILEHANDLER, could we make it more robust so that it could handle any 
source/destination file transfer? I think something along those lines was part 
of the original transfer_logs code.

thanks,
ginnie


On 02/22/11 10:56 AM, Keith Mitchell wrote:
[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


_______________________________________________
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
_______________________________________________
caiman-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/caiman-discuss

Reply via email to