classification
Title: logging.handlers.RotatingFileHandler - mode argument not respected
Type: behavior Stage:
Components: Documentation Versions: Python 3.1, Python 3.2, Python 2.7
process
Status: closed Resolution: not a bug
Dependencies: Superseder:
Assigned To: vinay.sajip Nosy List: fridrik, terry.reedy, vinay.sajip
Priority: normal Keywords: easy, patch

Created on 2010-08-04 15:38 by fridrik, last changed 2010-08-23 10:39 by fridrik. This issue is now closed.

Messages (10)
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) * (Python committer) 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) * (Python committer) 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) * (Python committer) 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) * (Python committer) 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) * (Python committer) 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'."
History
Date User Action Args
2010-08-23 10:39:33fridriksetmessages: + msg114735
2010-08-23 00:18:48vinay.sajipsetstatus: open -> closed
resolution: not a bug
messages: + msg114719
2010-08-22 18:35:30fridriksetstatus: pending -> open

messages: + msg114692
2010-08-22 18:34:58vinay.sajipsetstatus: open -> pending
2010-08-22 18:11:12vinay.sajipsetstatus: pending -> open

messages: + msg114686
2010-08-22 18:04:00vinay.sajipsetstatus: open -> pending

nosy: - docs@python
messages: + msg114685

assignee: docs@python -> vinay.sajip
2010-08-05 16:43:11terry.reedysetmessages: + msg112999
2010-08-05 12:33:07fridriksetmessages: + msg112975
2010-08-05 12:30:50fridriksetnosy: + vinay.sajip
messages: + msg112974
2010-08-04 23:28:38terry.reedysetassignee: docs@python
components: + Documentation, - Library (Lib)
versions: + Python 3.1, Python 3.2, - Python 2.6, Python 2.5
keywords: + patch, easy
nosy: + docs@python, terry.reedy

messages: + msg112921
2010-08-04 15:38:57fridrikcreate