-
Notifications
You must be signed in to change notification settings - Fork 666
Comments
Generalize the RotatingLogger; size, time or first of the two trigger the rollover#1365
Generalize the RotatingLogger; size, time or first of the two trigger the rollover #1365j-c-cook wants to merge 29 commits intohardbyte:main from
Conversation
- Adds a deprecation warning to the TimedRotatingLogger. The TimedRotatingLogger now inherits from the generalized RotatingLogger.
j-c-cook
commented
Aug 11, 2022
The BLFWriter will be able to be used like the following:
python -m can.logger -i socketcan -c vcan0 -b 250000 -f file.blf -t 600 --compression-level=1 --max-container-size=200000
Here, a rollover would occur every 5 seconds, or when the buffer and file size becomes 200000. Note that the default max_container_size is 128 * 1024.
Another option would be:
python -m can.logger -i socketcan -c vcan0 -b 250000 -f file.log -t 600 -s 100000
A rollover would occur every 5 seconds, or when the file size becomes 100000 bytes.
- When time is a constraint for the
RotatingLogger, don't rollover if there have been no messages received since the last rollover
This is inherent to the implementation of logger. When a message is received then the logger is called. The file cannot rollover without there being a message passed to logger.
Lines 246 to 251 in 88fc8e5
Codecov Report
Merging #1365 (22bf537) into develop (4b1acde) will increase coverage by
0.12%.
The diff coverage is100.00%.
❗ Current head 22bf537 differs from pull request most recent head cabafb4. Consider uploading reports for the commit cabafb4 to get more accurate results
Additional details and impacted files
@@ Coverage Diff @@ ## develop #1365 +/- ## =========================================== + Coverage 65.16% 65.29% +0.12% =========================================== Files 81 81 Lines 8853 8779 -74 =========================================== - Hits 5769 5732 -37 + Misses 3084 3047 -37
j-c-cook
commented
Sep 29, 2022
@zariiii9003 This RotatingLogger class has been successfully running on multiple devices, that I manage, for about a month now (prior to the refactoring I've done today). There have been tens and possibly hundreds of hours of testing on the class by now. It is ready for a review. Please let me know what you think, thanks.
@felixdivo
felixdivo
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Regarding the discussion in #1364: I generally think that separating what can be split is a fair goal. In this case, however, I think having one class for this makes a lot of sense since it does simplify both user code and the implementation (I think having cooperating sub-classes to have a logger that supports both variants of rotations will not be easier to understand than this).
There are also a few nice small improvements in here that should be merged regardless of the new features!
We should finish the talk about the general API in #1364 before merging here.
Co-authored-by: Felix Divo <4403130+felixdivo@users.noreply.github.com>
Here is what was stated in the warning message in the `docs` workflow: Warning, treated as error: /home/runner/work/python-can/python-can/can/io/logger.py:docstring of can.io.logger.RotatingLogger:6:py:attr reference target not found: namer Error: Process completed with exit code 2.
Here's what the error check said: Run mypy --python-version 3.7 . can/io/logger.py:487: error: "__init__" of "RotatingLogger" gets multiple values for keyword argument "max_bytes" [misc] Found 1 error in 1 file (checked 55 source files) Error: Process completed with exit code 1.
...c-cook/python-can into issue1364_TimedRotatingLogger
Uh oh!
There was an error while loading. Please reload this page.
A
RotatingLoggerobject is created that constrains the file by file size, time or both. In the case of when both are constrained, the first that occurs will trigger the rollover.RotatingLogger, don't rollover if there have been no messages received since the last rolloverRotatingLoggercloses #1364