| msg112821 - (view) |
Author: Friðrik Már Jónsson (fridrik) |
Date: 2010-08-04 15:38 |
| It seems to me that the ``mode`` keyword argument of ``logging.handlers.RotatingFileHandler`` is not respected. Here is an example of opening a nonexistent file:: Python 2.7 (r27:82500, Aug 4 2010, 15:10:49) [GCC 4.3.2] on linux2 Type "help", "copyright", "credits" or "license" for more information. >>> import logging.handlers >>> handler = logging.handlers.RotatingFileHandler('nonexistent.log', ... mode='a+', maxBytes=1000, backupCount=2) >>> handler.mode 'a' >>> handler.stream <open file '/home/fridrik/nonexistent.log', mode 'a' at 0x2aefbca796f0> The docs do not mention any deviations from the behavior I expected (``handler.stream`` having the mode specified as an argument to ``RotatingFileHandler``); only that "If mode is not specified, 'a' is used." I've confirmed the same behavior on 2.5.2 and 2.6.2. This happens regardless of whether the file is being created or already exists. |
|
|
| msg112921 - (view) |
Author: Terry J. Reedy (terry.reedy) *  |
Date: 2010-08-04 23:28 |
| I presume your report is about the fact that the mode is 'a' even though you specified 'a+'. The answer is this block from RotatingFileHandler.__init__ (3.1.2, but presume same for 2.x): if maxBytes > 0: mode = 'a' # doesn't make sense otherwise! I do not understand the comment, but there it is. So DOC PATCH In 15.6.12.5. RotatingFileHandler, replace "If mode is not specified, 'a' is used." with "If mode is not specified or if maxBytes > 0, the mode is 'a'." |
|
|
| msg112974 - (view) |
Author: Friðrik Már Jónsson (fridrik) |
Date: 2010-08-05 12:30 |
| Thank you. I should have been more clear about what I meant. This this condition was introduced in r38631 by Vinay Sajip having the log message "Added optional encoding argument to file handlers." I can't easily see why this piece of code is necessary, which absolutely doesn't mean it isn't. I'm going to suggest some cases where other modes may be appropriate for loggers. This is a little open-ended and devoid of solution-orientation, but I don't know the rationale well enough to suggest alternatives. We do know that 'r' (read-only logger) and 'w' (logger rarely rolls over for traditional maxBytes values, but might -- this mode also makes relatively little sense with ``logger.FileHandler``) make little sense here. I'm not aware of a binary log format, so 'b' might make little sense as well. What about 'U' and '+'? For instance, the W3C Extended Log File Format draft uses headers at the beginning of a log file. Ideally, knowing whether to write headers to the file is done by using the ``handler.stream`` to determine whether the file is empty. It may be debatable whether supporting such formats is the purpose of handlers. If not, it's at the cost of writing new libraries that handle logging for those formats. I will never be able to exhaustively list use cases. If these modes are safe, shouldn't the developer decide makes sense, as long as it doesn't break the functionality of the logger? I don't know whether it's generally approrpiate to add people to the nosy list. I apologise, Vinay, if that's not common practice. |
|
|
| msg112975 - (view) |
Author: Friðrik Már Jónsson (fridrik) |
Date: 2010-08-05 12:33 |
| It may not have been entirely obvious that what I meant with the Extended Log File Format example is that read access would be optimal. |
|
|
| msg112999 - (view) |
Author: Terry J. Reedy (terry.reedy) * |
Date: 2010-08-05 16:43 |
| Given that you decided to argue for a code change (which my comment implicitly invited), adding someone (VS in this case) to nosy to respond is the right thing to do and fairly common. Assigning to someone is not, as least not any more. If someone agrees to change the code, the headers can be changed again. |
|
|
| msg114685 - (view) |
Author: Vinay Sajip (vinay.sajip) * |
Date: 2010-08-22 18:03 |
| Sorry for the slow response, I've been on vacation. The reason why the mode is set to "a" for rotating file handlers is as follows: the file is supposed to contain events from multiple runs of the program until rollover occurs. If a mode "w" (for example) is specified and respected, and you run the application twice, you would only get log output from the last run - not what you would intuitively expect. That's because in the second run, "w" would truncate an existing log file. With the mode set to "a", logs from multiple runs go into the same file, until rollover occurs - this seems to be the commonest use case. Why do you need to specify "a+" rather than "a"? Do you need to read the log file concurrently with it being written, with the same file handle/descriptor? The provided handlers are for text files, so non-text modes are not appropriate either. If you need support for binary files, you can create your own handler classes, perhaps based on the existing ones. Marking as pending, awaiting feedback. Will close in a couple of weeks if none received. |
|
|
| msg114686 - (view) |
Author: Vinay Sajip (vinay.sajip) * |
Date: 2010-08-22 18:11 |
| I've updated the comment to be more informative in py3k, release27-maint (r84259). |
|
|
| msg114692 - (view) |
Author: Friðrik Már Jónsson (fridrik) |
Date: 2010-08-22 18:35 |
| I agree with your points on the triviality and potential harmfulness of allowing modes like 'b' and 'w'. The '+' mode may be required for loggers that require headers or validation or positioning within an existing file (think XML). One example of such a format is the "Extended Log File Format" draft by the W3C which has seen widespread usage, despite being a draft. With a format like the ELFF, it must be determined on opening an existing file (perhaps amongst other sanitary operations), whether its headers have already been written or whether they need to be. It may also be desirable to simply check the file's size through its already open handler. I can see an argument for not allowing this into the handler; handlers are perhaps not meant to support formats but much rather storage mechanisms. Accepting this argument requires accepting the fact that libraries implementing such formats, using rotation and needing to read will need to write a handler from scratch for rotation support. There may be more uses of different modes than my single example. I see this boiling down to a matter of philosophy -- do we support usages we cannot foresee or do we admit certain features only when we've seen several valid examples of their use? Perhaps we should also consider what harm is possibly done by allowing these modes, and whether harmful use is likelier to be attributed to bad architecture or programmer recklessness. |
|
|
| msg114719 - (view) |
Author: Vinay Sajip (vinay.sajip) * |
Date: 2010-08-23 00:18 |
| The rotating handlers provided with the logging package are expected to be simple append-only handlers; if you need more specialised behaviour you need to subclass and implement what you need. The ELFF is about request logging, not application logging. Using the YAGNI principle, we don't implement features for which there is no apparent need. As there is no apparent need to support the a+ mode in a rotating handler in Python (since you have the ability to subclass and override), I'll close this issue. |
|
|
| msg114735 - (view) |
Author: Friðrik Már Jónsson (fridrik) |
Date: 2010-08-23 10:39 |
| That's a fair conclusion, but in this case I'd appreciate Terry's suggested doc patch being implemented: DOC PATCH In 15.6.12.5. RotatingFileHandler, replace "If mode is not specified, 'a' is used." with "If mode is not specified or if maxBytes > 0, the mode is 'a'." |
|
|