Fix logging rotation - #867
Conversation
oliver-sanders
left a comment
There was a problem hiding this comment.
This generally looks good. The standalone and multi-user files are now neatly separated and the rollover is working nicely - no more blank files or other nonsense.
I'm happy with the removal of the symlink, getting it to rename the file instead is perfectly reasonable.
I spotted one issue, it will fall over with traceback if the ~/.cylc/uiserver/log directory doesn't exist when the server is started. We should mkdir -p that at __init__ time.
This does change the filenames to remove the .log extension which is unfortunate as this naming is now inconsistent with other Cylc logs. I'm guessing it changed because that's the logging handler default, but I think we can override the rotation naming via the rotation_filename method or namer attr?
I think as it stands, this will leave any pre-existing <x>-uiserver.log files kicking around (as they are now out of the rotation), which is annoying. I think we could solve this by restoring the original file naming (which would pull the old files back into the rotation), or potentially by shimming something into the rollover code to wipe out files with the previous name (if there's a desire to rename the file for other reasons).
| # Class attribute to track whether logging has been started before. | ||
| # This is needed as Traitlets may reconfigure logging several times | ||
| # within the lifetime of the application, which creates new instances | ||
| # of the handler. | ||
| started = False |
There was a problem hiding this comment.
This is a little icky, but I'm guessing we have no access to application lifecycle information (or even the application itself) at this point so needs must.
It might be clearer to implement this as a global variable as it otherwise looks like the pattern of setting variable default on the class to be shadowed on the application.
The |
Done |
This seems to do the trick: import logging
import logging.handlers
from pathlib import Path
from time import time
class MyRotatingFileHandler(logging.handlers.RotatingFileHandler):
def rotation_filename(self, default_name):
"""
Modify the filename of a log file when rotating.
This is provided so that a custom filename can be provided.
The default implementation calls the 'namer' attribute of the
handler, if it's callable, passing the default name to
it. If the attribute isn't callable (the default is None), the name
is returned unchanged.
:param default_name: The default name for the log file.
"""
name, number = default_name.rsplit(".", 1)
path = Path(name)
return str(path.parent / f"{number}-{path.name}")
LOG = logging.getLogger("a")
LOG.addHandler(MyRotatingFileHandler("logfile.log", maxBytes=100, backupCount=2))
LOG.setLevel(logging.INFO)
LOG.info(f"Hello world {time()} ...........................................")But we do also have |
Supersedes and contains two of the bug fixes from #789
Closes #742, closes #743
Fixed logging issues:
cylc guiand JupyterHub - the latter now useshubapp-loginstead oflog.log.1,log.2etc. (orhubapp-log.1,hubapp-log.2etc.).Check List
CONTRIBUTING.mdand added my name as a Code Contributor.?.?.xbranch.