Adding support for rotation on a fixed number of logs

[lstep]

I propose the addition of a supplementary class to the logfile.py file. The 
problem with the current LogFile class is that the number of files continually 
grows (which may be useful, but not always). So I added a FixedLogFile class 
which takes a supplementary 'nbLogFiles' parameter, where you specify the 
maximum number of files to store on disk. 
Here is the diff file:

[therve]

Ready to review in logfile-1095.

I preferred to use an option to actual LogFile because it was more straightforward. Test and usual cleanups added.

[exarkun]

I suspect there's a better way to factor the implementation of all the features LogFile provides, this new one included. Something involving layers of objects wrapping others, each adding something, like maximum size and umask, seems like it would be a good idea. Not that I think this branch should refactor all this code, but seeing the parameter list for LogFile.init spill onto a second line made me think about this. :)

I think None would be a better choice for a don't-limit value than -1. The docstring language talking about whether maxNumberFiles "is set" or not is also a little confusing. If the don't-limit value is None, then these docs can just talk about whether the value "is None" or "is not None", which has a definite meaning in Python and so is easier to understand.

maxNumberFiles might also be a clumsier name for this parameter than is necessary. A few other names occur to me, maxRotatedFiles, maxFileCount, maxFileNumber, rotatedFileLimit, fileRotationLimit, etc.

It's probably also worth documenting that log files numbered above the rotation limit will be deleted. I think this is the right behavior, but it might be surprising to someone switching to a limited number of rotations.

Much as I am loathe to suggest adding further options to twistd related to logging which don't actually generally solve the problem of configuring the logging system, it does seem as though this feature is going to be of minimal benefit until it is actually exposed to twistd. Maybe this should be a separate ticket, though?

Aside from -1/None and the naming of the new parameter, the implementation and test look fine.

[therve]

Thanks for the new name for the parameter, I was hoping the reviewer whould find something better :). maxRotatedFiles looked good.

I'd be glad to improve twistd. Me prefered way would be to make this in the tac file, so we could define a LogObserver, like itamar proposed in #638. There is also #823 around. I'll look at #638.

For now, ready to review.

[dreid]

test_maxRotatedLogFiles is rather dense some more whitespace between the assertions might improve readability. I think self.assert_ is less clear than self.failUnless but that's a probably a personal preference, and I won't block the merge because of it.

Thanks for adding the docstrings, go ahead and merge.

[therve]

in [19589]

Merge logfile-1095

Author: therve Reviewer: dreid, exarkun

Add a new option maxRotatedFiles to LogFile to limit the number of files created with rotation.

[marc]

This is a really nice enhancement.

It would be nice to also implement for DailyLogfile.

[exarkun]

That would be fine. Please open a new ticket for it.