Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Log file rotation not rotating file #58

Closed
turner-prize opened this issue Feb 18, 2019 · 9 comments
Closed

Log file rotation not rotating file #58

turner-prize opened this issue Feb 18, 2019 · 9 comments
Labels
enhancement Improvement to an already existing feature

Comments

@turner-prize
Copy link

turner-prize commented Feb 18, 2019

I have a log configured as per the below:

logger.add("path/to/log.log",
format="{time:DD/MM/YY HH:mm:ss} - {message}",
level="CustomLogLevel",
filter=lamba record: record['level']="CustomLogLevel",
rotation="1 week")

The script which the above logger is linked to runs once per hour, for about 30 seconds. All works fine except the rotation, it's been configured since the 6th of Feb and hasn't created a new file yet, just keeps appending to the file which was initially created. Is this configured wrong on my end?

Edit:
Just read the contributions page!
using:
Python 3.7.2
Loguru 0.2.5
Windows Server 2016
Using both VS Code and terminal.

@turner-prize
Copy link
Author

Think I might have solved this one myself. I guess I wanted retention rather than rotation?

As per the docstrings in _logger.py;

  Parameters
    ----------
    rotation : |str|, |int|, |time|, |timedelta| or |function|_, optional
        A condition indicating whenever the current logged file should be closed and a new one
        started.

My file is opened and closed each time the file is run, I believe the above refers to a file being open for an extended period of time (in my case, a week) before the rotation will close and start a new one? In which case it won't apply to my situation?

@Delgan
Copy link
Owner

Delgan commented Feb 18, 2019

Hi @turner-prize.

As you understood, rotation is mainly used for applications running continuously. If it started since 1 week (as for your example), then the log file will be closed and a new one created.

The retention parameter is used to remove old log files. However, this only makes sense if you start a new log file each time you launch your application, that is if you use logger.add("file_{time}.log") for example.
That way, instead of creating "file_2019-02-18_12-00-00.log", "file_2019-02-18_13-00-00.log", "file_2019-02-18_14-00-00.log"... indefinitely, this only keeps the most recent ones.

There is currently no way to rotate your log file based on the time it was created, supposing you open and close the same file each time you run your application.
Maybe I could change the behavior of the rotation argument, so it no longer uses the time the application is launched, but rather the time the file was first created. 🤔

@turner-prize
Copy link
Author

Ah that makes sense. I should have read the docs a bit closer! I might just add a function before the logger config to check how old the log is, and rename it with an appended date if over a week old, which will do the job.

Love the library by the way!

@Delgan
Copy link
Owner

Delgan commented Feb 18, 2019

You could also pass your function to the rotation, argument. Something like this should work I think (not tested):

def should_rotate(message, file):
    filepath = os.path.abspath(file.name)
    creation = os.path.getmtime(filepath)
    now = message.record["time"].timestamp()
    maxtime = 7 * 24 * 60 * 60  # 1 week in seconds
    return now - creation > maxtime

logger.add("file.log", rotation=should_rotate)

Love the library by the way!

Thanks!

@Delgan
Copy link
Owner

Delgan commented Mar 3, 2019

I'm reopening this issue as I think I may implement the workaround directly into Loguru.

It makes more sense to have the rotation argument based on the file date creation rather that the application started time.

@Delgan Delgan reopened this Mar 3, 2019
@Delgan Delgan added the enhancement Improvement to an already existing feature label Mar 3, 2019
@JonasR-
Copy link

JonasR- commented Mar 5, 2019

I am testing loguru currently and was confused that the rotation behaves like this. I appreciate that you will implement the rotation based on creation date

@YJinHai
Copy link

YJinHai commented Apr 4, 2019

I'm reopening this issue as I think I may implement the workaround directly into Loguru.

It makes more sense to have the rotation argument based on the file date creation rather that the application started time.

I think so, too. Thank you

@YJinHai
Copy link

YJinHai commented Apr 4, 2019

您也可以将函数传递给rotation参数。我认为这样的事情应该有效(未经测试):

def  should_rotate消息文件):
    filepath = os.path.abspathfile .namecreation = os.path.getmtimefilepathnow = message.record [ “ time ” ] .timestamp()
    MAXTIME =  7  *  24  *  60  *  601周秒
    返回现在-创建> MAXTIME

logger.add( “ file.log ”,旋转= should_rotate

顺便爱上图书馆!

谢谢!

You could also pass your function to the rotation, argument. Something like this should work I think (not tested):

def should_rotate(message, file):
    filepath = os.path.abspath(file.name)
    creation = os.path.getmtime(filepath)
    now = message.record["time"].timestamp()
    maxtime = 7 * 24 * 60 * 60  # 1 week in seconds
    return now - creation > maxtime

logger.add("file.log", rotation=should_rotate)

Love the library by the way!

Thanks!
I found a small error here. os.path.getmtime (path) returns the most recent file modification time
os.path.getctime (path) returns the file path creation time, which should be changed from getmtime to getctime

@Delgan
Copy link
Owner

Delgan commented Jun 8, 2019

Ok, I finally find the time to update the rotation behavior based on your suggestion! This should be effective once the v0.3.0 is published.

As @YJinHai correctly noticed, the first snipped I provided was wrong because based on the mtime attribute (which corresponds to the time of the last modification, not the time of the file creation).

Actually, there is no standard cross-platform way to retrieve the creation time of a file. Python doesn't provide such utility and it's not easily accessible on Linux systems. See this question on SO.
Windows too has a problem. While rotating "file.log", it needs to be renamed to "file.<time>.log" and a new "file.log" re-created. This trigger the "file system tunneling" feature of Microsoft, which means the new "file.log" re-use the creation time of the previously renamed file. This hence prevents time-based rotation to properly make use of st_ctime.

Consequently, I had to implement a different policy on each platform:

This should work in most cases.

@Delgan Delgan closed this as completed Jun 8, 2019
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Improvement to an already existing feature
Projects
None yet
Development

No branches or pull requests

4 participants