[Twisted-Python] twisted.lumberjack clobbers existing logs
Hi everyone... I like the twisted.lumberjack module that was recently added, but I think it has a small buglet. It checks the size of the existing log file, but then opens it in mode "w", truncating it. Thus, I suggest the last line of LogFile._openFile should change from: self._file = open(self.path, "w") to: self._file = open(self.path, "a") -Andrew.
Andrew Bennetts wrote:
I like the twisted.lumberjack module that was recently added, but I think it has a small buglet. It checks the size of the existing log file, but then opens it in mode "w", truncating it.
Thus, I suggest the last line of LogFile._openFile should change from: self._file = open(self.path, "w") to: self._file = open(self.path, "a")
Actually, "a" mode is a bad idea, since according to the docs has inconsistent behaviour: "on *some* Unix systems means that *all* writes append to the end of the file, regardless of the current seek position)." But you're write about that. I'll fix it ASAP. Thanks for the bug report!
On Mon, Feb 25, 2002 at 09:50:39AM +0000, Itamar Shtull-Trauring wrote:
Actually, "a" mode is a bad idea, since according to the docs has inconsistent behaviour: "on *some* Unix systems means that *all* writes append to the end of the file, regardless of the current seek position)."
I considered that, but when would a log file ever want to do otherwise? I'd be interested to know what a portable way to append to a file is, though. -Andrew.
Andrew Bennetts wrote:
I considered that, but when would a log file ever want to do otherwise?
"a" also screws up seek()s, which I'll need for the remote log viewing service. In general, "a" seems like a bad idea since its behavior is inconsistent.
I'd be interested to know what a portable way to append to a file is, though.
# assuming "file" exists f = open("file", "r+") f.seek(0, 2) see the code I checked in for a complete example.
On Mon, Feb 25, 2002 at 05:27:33PM +0000, Itamar Shtull-Trauring wrote:
Andrew Bennetts wrote:
I considered that, but when would a log file ever want to do otherwise?
"a" also screws up seek()s, which I'll need for the remote log viewing service. In general, "a" seems like a bad idea since its behavior is inconsistent.
Fair enough.
I'd be interested to know what a portable way to append to a file is, though.
# assuming "file" exists
f = open("file", "r+")
f.seek(0, 2)
see the code I checked in for a complete example.
Ah, of course. Thanks. -Andrew.
On Mon, Feb 25, 2002 at 05:27:33PM +0000, Itamar Shtull-Trauring wrote:
# assuming "file" exists
f = open("file", "r+")
f.seek(0, 2)
see the code I checked in for a complete example.
Well, this assumes that there is only one writer for the file. O_APPEND OTOH, when implemented in the kernel (and not emulated with the above code in the library), makes this as ONE atomic call. (basically, it seeks to the end of the file before any write).
From open(2): O_APPEND The file is opened in append mode. Before each write, the file pointer is positioned at the end of the file, as if with lseek. O_APPEND may lead to corrupted files on NFS file systems if more than one process appends data to a file at once. This is because NFS does not support appending to a file, so the client kernel has to simulate it, which can't be done without a race condition.
Basically, in the NFS case the kernel is not able to do it atomic so there might be problems. Side note: It's quite acceptable to use seek/read with an O_RDWR|O_APPEND file. It's just automatically seeks to the end before writing. So O_APPEND is quite useful. Andreas
Andreas Kostyrka wrote:
Well, this assumes that there is only one writer for the file.
The code in twisted.python.log assures that there is only one writer at any given time. We want to do this on the Python side, not using OS features, in order to make the code as portables as possible.
Yepp, http://www.twistedmatrix.com/foo/ does the exception-dance, but not http://www.twistedmatrix.com/foo obviously. So, this isn't anything terrible and hard, but I just couldn't recreate the situation here (win2k). Probably fixed, but somebody ought to know. mvh. Benjamin Bruheim Teknisk Leder, In/Out Bergen :: www.inout.no
participants (5)
-
Andreas Kostyrka
-
Andrew Bennetts
-
Andrew Bennetts
-
Benjamin Bruheim
-
Itamar Shtull-Trauring