From 5c232483e97d92b002886ada109f4f6500ffec25 Mon Sep 17 00:00:00 2001 From: j-c-cook Date: Sun, 7 Aug 2022 17:01:04 -0500 Subject: [PATCH 01/22] Generalize the RotatingLogger; size, time or both constraint - Adds a deprecation warning to the TimedRotatingLogger. The TimedRotatingLogger now inherits from the generalized RotatingLogger. --- can/__init__.py | 9 +++- can/io/__init__.py | 2 +- can/io/logger.py | 103 +++++++++++++++++++++++++++++++++++++++++---- can/logger.py | 22 +++++++--- 4 files changed, 121 insertions(+), 15 deletions(-) diff --git a/can/__init__.py b/can/__init__.py index 2a0b805ac..765328296 100644 --- a/can/__init__.py +++ b/can/__init__.py @@ -35,7 +35,14 @@ from .interface import Bus, detect_available_configs from .bit_timing import BitTiming -from .io import Logger, SizedRotatingLogger, Printer, LogReader, MessageSync +from .io import ( + Logger, + SizedRotatingLogger, + Printer, + LogReader, + MessageSync, + RotatingLogger, +) from .io import ASCWriter, ASCReader from .io import BLFReader, BLFWriter from .io import CanutilsLogReader, CanutilsLogWriter diff --git a/can/io/__init__.py b/can/io/__init__.py index 0d3741b05..76eca39ac 100644 --- a/can/io/__init__.py +++ b/can/io/__init__.py @@ -4,7 +4,7 @@ """ # Generic -from .logger import Logger, BaseRotatingLogger, SizedRotatingLogger +from .logger import Logger, BaseRotatingLogger, SizedRotatingLogger, RotatingLogger from .player import LogReader, MessageSync # Format specific diff --git a/can/io/logger.py b/can/io/logger.py index 37ba1e736..88b819f09 100644 --- a/can/io/logger.py +++ b/can/io/logger.py @@ -6,6 +6,8 @@ import pathlib from abc import ABC, abstractmethod from datetime import datetime +import time +import warnings import gzip from typing import Any, Optional, Callable, Type, Tuple, cast, Dict @@ -259,7 +261,7 @@ def do_rollover(self) -> None: """Perform rollover.""" -class SizedRotatingLogger(BaseRotatingLogger): +class RotatingLogger(BaseRotatingLogger): """Log CAN messages to a sequence of files with a given maximum size. The logger creates a log file with the given `base_filename`. When the @@ -299,38 +301,57 @@ class SizedRotatingLogger(BaseRotatingLogger): :meth:`~can.Listener.stop` is called. """ + last_rollover_time = time.time() + def __init__( self, base_filename: StringPathLike, *args: Any, max_bytes: int = 0, + delta_t: int = 0, **kwargs: Any, ) -> None: """ :param base_filename: - A path-like object for the base filename. The log file format is defined by - the suffix of `base_filename`. + A path-like object for the base filename. The log file format is + defined by the suffix of `base_filename`. :param max_bytes: - The size threshold at which a new log file shall be created. If set to 0, no - rollover will be performed. + The size threshold at which a new log file shall be created. If set + less than or equal to 0, no rollover will be performed. + :param delta_t: + The elapsed time threshold at which a new log file shall be + created. If set less than or equal to 0, no rollover will be + performed. """ super().__init__(*args, **kwargs) self.base_filename = os.path.abspath(base_filename) - self.max_bytes = max_bytes + + # Rotation parameters + self.max_bytes = max_bytes # Maximum bytes for rotation (bytes) + self.delta_t = delta_t # Time difference between rotation (seconds) self._writer = self._get_new_writer(self.base_filename) def should_rollover(self, msg: Message) -> bool: - if self.max_bytes <= 0: + # Check to see if a file rollover should occur based on file size + # (bytes) and elapsed time (seconds) since last rollover. + if self.max_bytes <= 0 and self.delta_t <= 0: return False - if self.writer.file.tell() >= self.max_bytes: + # Check to see if the file size is greater than max bytes + if self.writer.file.tell() >= self.max_bytes > 0: + return True + # Check to see if elapsed time is greater than delta_t + now = time.time() + if now - self.last_rollover_time > self.delta_t > 0: + self.last_rollover_time = now return True return False def do_rollover(self) -> None: + # Perform the file rollover. if self.writer: self.writer.stop() @@ -352,3 +373,69 @@ def _default_name(self) -> StringPathLike: + path.suffix ) return str(path.parent / new_name) + + +class SizedRotatingLogger(RotatingLogger): + """Log CAN messages to a sequence of files with a given maximum size. + + The logger creates a log file with the given `base_filename`. When the + size threshold is reached the current log file is closed and renamed + by adding a timestamp and the rollover count. A new log file is then + created and written to. + + This behavior can be customized by setting the :attr:`namer` and + :attr:`rotator` attribute. + + Example:: + + from can import Notifier, SizedRotatingLogger + from can.interfaces.vector import VectorBus + + bus = VectorBus(channel=[0], app_name="CANape", fd=True) + + logger = SizedRotatingLogger( + base_filename="my_logfile.asc", + max_bytes=5 * 1024 ** 2, # =5MB + ) + logger.rollover_count = 23 # start counter at 23 + + notifier = Notifier(bus=bus, listeners=[logger]) + + The SizedRotatingLogger currently supports the formats + * .asc: :class:`can.ASCWriter` + * .blf :class:`can.BLFWriter` + * .csv: :class:`can.CSVWriter` + * .log :class:`can.CanutilsLogWriter` + * .txt :class:`can.Printer` (if pointing to a file) + + .. note:: + The :class:`can.SqliteWriter` is not supported yet. + + The log files on disk may be incomplete due to buffering until + :meth:`~can.Listener.stop` is called. + """ + + def __init__( + self, + base_filename: StringPathLike, + *args: Any, + max_bytes: int = 0, + **kwargs: Any, + ) -> None: + """ + :param base_filename: + A path-like object for the base filename. The log file format is defined by + the suffix of `base_filename`. + :param max_bytes: + The size threshold at which a new log file shall be created. If set to 0, no + rollover will be performed. + """ + # This object is deprecated as of 4.1 and will be removed in 5.0. + warnings.simplefilter("always", DeprecationWarning) + warnings.warn( + "SizedRotatingLogger is being replaced with the " + "generalized RotatingLogger in python-can 5.0.", + DeprecationWarning, + ) + # Initialize self as a RotatingLogger + super().__init__(base_filename, *args, max_bytes=max_bytes, **kwargs) diff --git a/can/logger.py b/can/logger.py index 209c1381c..782510278 100644 --- a/can/logger.py +++ b/can/logger.py @@ -21,7 +21,7 @@ from typing import Any, Dict, List, Union, Sequence, Tuple import can -from . import Bus, BusState, Logger, SizedRotatingLogger +from . import Bus, BusState, Logger, SizedRotatingLogger, RotatingLogger from .typechecking import CanFilter, CanFilters @@ -193,7 +193,16 @@ def main() -> None: type=int, help="Maximum file size in bytes. Rotate log file when size threshold " "is reached.", - default=None, + default=0, + ) + + parser.add_argument( + "-t", + "--time_difference", + dest="delta_t", + type=int, + help="Time difference in seconds between file rollover.", + default=0, ) parser.add_argument( @@ -235,9 +244,12 @@ def main() -> None: print(f"Can Logger (Started on {datetime.now()})") options = {"append": results.append} - if results.file_size: - logger = SizedRotatingLogger( - base_filename=results.log_file, max_bytes=results.file_size, **options + if results.file_size or results.delta_t: + logger = RotatingLogger( + base_filename=results.log_file, + max_bytes=results.file_size, + delta_t=results.delta_t, + **options, ) else: logger = Logger(filename=results.log_file, **options) # type: ignore From 019e41c14dd91d885afd672dd85c2b8fcc7c0e10 Mon Sep 17 00:00:00 2001 From: j-c-cook Date: Thu, 11 Aug 2022 10:53:31 -0500 Subject: [PATCH 02/22] Update test_log_virtual_sizedlogger to have a file path --- test/test_logger.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/test_logger.py b/test/test_logger.py index bb0015a89..081ddcb0a 100644 --- a/test/test_logger.py +++ b/test/test_logger.py @@ -103,7 +103,7 @@ def test_log_virtual_sizedlogger(self): self.MockLoggerUse = self.MockLoggerSized self.loggerToUse = self.mock_logger_sized - sys.argv = self.baseargs + ["--file_size", "1000000"] + sys.argv = self.baseargs + ["-f file.log"] + ["--file_size", "1000000"] can.logger.main() self.assertSuccessfullCleanup() self.mock_logger_sized.assert_called_once() From e091ba0005d5773c0eaab047408c28cb63cbeea1 Mon Sep 17 00:00:00 2001 From: j-c-cook Date: Thu, 11 Aug 2022 13:22:55 -0500 Subject: [PATCH 03/22] Update logger doc string --- can/io/logger.py | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/can/io/logger.py b/can/io/logger.py index 734838f16..be4c2756c 100644 --- a/can/io/logger.py +++ b/can/io/logger.py @@ -263,10 +263,13 @@ def do_rollover(self) -> None: class RotatingLogger(BaseRotatingLogger): - """Log CAN messages to a sequence of files with a given maximum size. + """Log CAN messages to a sequence of files with a given maximum size, + a specified amount of time or a combination of both. When both max size + (bytes) and rollover time (seconds) are provided, the rollover trigger + is caused by the first to occur. - The logger creates a log file with the given `base_filename`. When the - size threshold is reached the current log file is closed and renamed + The logger creates a log file with the given `base_filename`. When a size + or time threshold is reached the current log file is closed and renamed by adding a timestamp and the rollover count. A new log file is then created and written to. From 5be3a81d32efb0c360afa824dd33ae024097c391 Mon Sep 17 00:00:00 2001 From: j-c-cook Date: Thu, 11 Aug 2022 16:08:50 -0500 Subject: [PATCH 04/22] Move rollover time reset to capture when file_size triggers --- can/io/logger.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/can/io/logger.py b/can/io/logger.py index be4c2756c..76e7fee71 100644 --- a/can/io/logger.py +++ b/can/io/logger.py @@ -347,9 +347,7 @@ def should_rollover(self, msg: Message) -> bool: if self.writer.file_size() >= self.max_bytes > 0: return True # Check to see if elapsed time is greater than delta_t - now = time.time() - if now - self.last_rollover_time > self.delta_t > 0: - self.last_rollover_time = now + if time.time() - self.last_rollover_time > self.delta_t > 0: return True return False @@ -359,6 +357,9 @@ def do_rollover(self) -> None: if self.writer: self.writer.stop() + # Reset the time since last rollover + self.last_rollover_time = time.time() + sfn = self.base_filename dfn = self.rotation_filename(self._default_name()) self.rotate(sfn, dfn) From 0cf83839a217dae6dd958d20d779bd63e88280cd Mon Sep 17 00:00:00 2001 From: j-c-cook Date: Thu, 29 Sep 2022 09:31:48 -0500 Subject: [PATCH 05/22] Change `delta_t` to `max_seconds` --- can/io/logger.py | 12 ++++++------ can/logger.py | 4 ++-- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/can/io/logger.py b/can/io/logger.py index 99dbac634..37420b328 100644 --- a/can/io/logger.py +++ b/can/io/logger.py @@ -317,7 +317,7 @@ def __init__( base_filename: StringPathLike, *args: Any, max_bytes: int = 0, - delta_t: int = 0, + max_seconds: int = 0, **kwargs: Any, ) -> None: """ @@ -327,7 +327,7 @@ def __init__( :param max_bytes: The size threshold at which a new log file shall be created. If set less than or equal to 0, no rollover will be performed. - :param delta_t: + :param max_seconds: The elapsed time threshold at which a new log file shall be created. If set less than or equal to 0, no rollover will be performed. @@ -338,21 +338,21 @@ def __init__( # Rotation parameters self.max_bytes = max_bytes # Maximum bytes for rotation (bytes) - self.delta_t = delta_t # Time difference between rotation (seconds) + self.max_seconds = max_seconds # Time difference between rotation (seconds) self._writer = self._get_new_writer(self.base_filename) def should_rollover(self, msg: Message) -> bool: # Check to see if a file rollover should occur based on file size # (bytes) and elapsed time (seconds) since last rollover. - if self.max_bytes <= 0 and self.delta_t <= 0: + if self.max_bytes <= 0 and self.max_seconds <= 0: return False # Check to see if the file size is greater than max bytes if self.writer.file_size() >= self.max_bytes > 0: return True - # Check to see if elapsed time is greater than delta_t - if time.time() - self.last_rollover_time > self.delta_t > 0: + # Check to see if elapsed time is greater than max seconds + if time.time() - self.last_rollover_time > self.max_seconds > 0: return True return False diff --git a/can/logger.py b/can/logger.py index 811390a48..cbd6f70b1 100644 --- a/can/logger.py +++ b/can/logger.py @@ -202,7 +202,7 @@ def main() -> None: parser.add_argument( "-t", "--time_difference", - dest="delta_t", + dest="max_seconds", type=int, help="Time difference in seconds between file rollover.", default=0, @@ -251,7 +251,7 @@ def main() -> None: logger = RotatingLogger( base_filename=results.log_file, max_bytes=results.file_size, - delta_t=results.delta_t, + max_seconds=results.max_seconds, append=results.append, **additional_config, ) From cd369959b8ffa7094b8fc7b23a71f9e98554492a Mon Sep 17 00:00:00 2001 From: j-c-cook Date: Thu, 29 Sep 2022 09:47:10 -0500 Subject: [PATCH 06/22] Run logger test on `RotatingLogger` rather than `SizedRotatingLogger` --- test/test_logger.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/test_logger.py b/test/test_logger.py index 081ddcb0a..ebde30274 100644 --- a/test/test_logger.py +++ b/test/test_logger.py @@ -39,7 +39,7 @@ def setUp(self) -> None: self.loggerToUse = self.mock_logger # Patch SizedRotatingLogger object - patcher_logger_sized = mock.patch("can.logger.SizedRotatingLogger", spec=True) + patcher_logger_sized = mock.patch("can.logger.RotatingLogger", spec=True) self.MockLoggerSized = patcher_logger_sized.start() self.addCleanup(patcher_logger_sized.stop) self.mock_logger_sized = self.MockLoggerSized.return_value From c9ba894cc4226f705529508337736775d17d264e Mon Sep 17 00:00:00 2001 From: j-c-cook Date: Thu, 29 Sep 2022 09:57:17 -0500 Subject: [PATCH 07/22] Modify timeout period name and call Rotating if `file_time` --- can/logger.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/can/logger.py b/can/logger.py index cbd6f70b1..2d4e1f736 100644 --- a/can/logger.py +++ b/can/logger.py @@ -201,10 +201,10 @@ def main() -> None: parser.add_argument( "-t", - "--time_difference", - dest="max_seconds", + "--file_time", + dest="file_time", type=int, - help="Time difference in seconds between file rollover.", + help="Maximum period in seconds before rotating log file.", default=0, ) @@ -247,11 +247,11 @@ def main() -> None: print(f"Can Logger (Started on {datetime.now()})") logger: Union[MessageWriter, BaseRotatingLogger] - if results.file_size: + if results.file_size or results.file_time: logger = RotatingLogger( base_filename=results.log_file, max_bytes=results.file_size, - max_seconds=results.max_seconds, + max_seconds=results.file_time, append=results.append, **additional_config, ) From dc75109db5387674a3eca8c911b6c0325f6451df Mon Sep 17 00:00:00 2001 From: j-c-cook Date: Thu, 29 Sep 2022 09:58:16 -0500 Subject: [PATCH 08/22] Add test for the timed logger given `file_time` input --- test/test_logger.py | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/test/test_logger.py b/test/test_logger.py index ebde30274..50c985fde 100644 --- a/test/test_logger.py +++ b/test/test_logger.py @@ -108,6 +108,16 @@ def test_log_virtual_sizedlogger(self): self.assertSuccessfullCleanup() self.mock_logger_sized.assert_called_once() + def test_log_virtual_timedlogger(self): + self.mock_virtual_bus.recv = Mock(side_effect=[self.testmsg, KeyboardInterrupt]) + self.MockLoggerUse = self.MockLoggerSized + self.loggerToUse = self.mock_logger_sized + + sys.argv = self.baseargs + ["-f file.log"] + ["--file_time", "5"] + can.logger.main() + self.assertSuccessfullCleanup() + self.mock_logger_sized.assert_called_once() + def test_parse_additional_config(self): unknown_args = [ "--app-name=CANalyzer", From 22bf537f23e3cabe9278524118f09fb761e62104 Mon Sep 17 00:00:00 2001 From: j-c-cook Date: Thu, 29 Sep 2022 10:04:34 -0500 Subject: [PATCH 09/22] Update logger help statement --- can/logger.py | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/can/logger.py b/can/logger.py index 2d4e1f736..33bd3dcd5 100644 --- a/can/logger.py +++ b/can/logger.py @@ -194,8 +194,9 @@ def main() -> None: dest="file_size", type=int, help="Maximum file size in bytes (or for the case of blf, maximum " - "buffer size before compression and flush to file). Rotate log " - "file when size threshold is reached.", + "buffer size before compression and flush to file). Rotate log file " + "when size threshold is reached. (If file_time is also given, then the " + "first of the two constraints to occur is what causes the rollover.)", default=0, ) @@ -204,7 +205,9 @@ def main() -> None: "--file_time", dest="file_time", type=int, - help="Maximum period in seconds before rotating log file.", + help="Maximum period in seconds before rotating log file. (If file_size" + "is also given, then the first of the two constraints to occur is" + "what causes the rollover.)", default=0, ) From c3d871506c0f6b71dbac8198b72095c87ba12877 Mon Sep 17 00:00:00 2001 From: Jack Cook Date: Wed, 28 Dec 2022 10:25:52 -0600 Subject: [PATCH 10/22] Update can/logger.py Co-authored-by: Felix Divo <4403130+felixdivo@users.noreply.github.com> --- can/logger.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/can/logger.py b/can/logger.py index fa0d27930..d1977d2f2 100644 --- a/can/logger.py +++ b/can/logger.py @@ -190,8 +190,8 @@ def main() -> None: "--file_time", dest="file_time", type=int, - help="Maximum period in seconds before rotating log file. (If file_size" - "is also given, then the first of the two constraints to occur is" + help="Maximum period in seconds before rotating log file. (If file_size " + "is also given, then the first of the two constraints to occur is " "what causes the rollover.)", default=0, ) From b6b75b97deb18668a9233762991eec20e088789e Mon Sep 17 00:00:00 2001 From: j-c-cook Date: Wed, 28 Dec 2022 10:49:39 -0600 Subject: [PATCH 11/22] Adjust the example in docstring and update documentation @felixdivo --- can/io/logger.py | 26 ++++++++++++++++++++++---- doc/listeners.rst | 2 +- 2 files changed, 23 insertions(+), 5 deletions(-) diff --git a/can/io/logger.py b/can/io/logger.py index bdd1afe5b..6deeba2b4 100644 --- a/can/io/logger.py +++ b/can/io/logger.py @@ -293,20 +293,38 @@ class RotatingLogger(BaseRotatingLogger): Example:: - from can import Notifier, SizedRotatingLogger + from can import Notifier, RotatingLogger from can.interfaces.vector import VectorBus bus = VectorBus(channel=[0], app_name="CANape", fd=True) - logger = SizedRotatingLogger( + # Size constrained rollover + logger = RotatingLogger( base_filename="my_logfile.asc", - max_bytes=5 * 1024 ** 2, # =5MB + max_bytes=5 * 1024 ** 2, # = 5 MB ) logger.rollover_count = 23 # start counter at 23 notifier = Notifier(bus=bus, listeners=[logger]) - The SizedRotatingLogger currently supports the formats + Example:: + + # Size or time constrained rollover (whichever limit occurs first) + logger = RotatingLogger( + base_filename="my_logfile.asc", + max_bytes=5 * 1024 ** 2, # = 5 MB + max_seconds=5 * 60, # = 5 minutes + ) + + Example:: + + # Time constrained rollover + logger = RotatingLogger( + base_filename="my_logfile.asc", + max_seconds=5 * 60, # = 5 minutes + ) + + The RotatingLogger currently supports the formats * .asc: :class:`can.ASCWriter` * .blf :class:`can.BLFWriter` * .csv: :class:`can.CSVWriter` diff --git a/doc/listeners.rst b/doc/listeners.rst index 260854d2a..d3c7e6726 100644 --- a/doc/listeners.rst +++ b/doc/listeners.rst @@ -97,7 +97,7 @@ create log files with different file types of the messages received. .. autoclass:: can.io.BaseRotatingLogger :members: -.. autoclass:: can.SizedRotatingLogger +.. autoclass:: can.RotatingLogger :members: From a830c7a736f39d9333d3745994cf06bd590de857 Mon Sep 17 00:00:00 2001 From: j-c-cook Date: Wed, 28 Dec 2022 10:56:53 -0600 Subject: [PATCH 12/22] Update the deprecation warning message --- can/io/logger.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/can/io/logger.py b/can/io/logger.py index 6deeba2b4..b73738827 100644 --- a/can/io/logger.py +++ b/can/io/logger.py @@ -472,11 +472,12 @@ def __init__( The size threshold at which a new log file shall be created. If set to 0, no rollover will be performed. """ - # This object is deprecated as of 4.1 and will be removed in 5.0. + # This object is deprecated as of 4.2 and will be removed in 5.0. warnings.simplefilter("always", DeprecationWarning) warnings.warn( - "SizedRotatingLogger is being replaced with the " - "generalized RotatingLogger in python-can 5.0.", + "`can.io.SizedRotatingLogger` is deprecated as of v4.2. It will be " + "removed in v5.0. New features are not supported by this object. " + "Use the `can.io.RotatingLogger`. class instead", DeprecationWarning, ) # Initialize self as a RotatingLogger From 2862573c09a2c7b2271169151642af9dcdfb6c27 Mon Sep 17 00:00:00 2001 From: j-c-cook Date: Wed, 28 Dec 2022 11:05:18 -0600 Subject: [PATCH 13/22] Fix doc string build warning 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. --- can/io/logger.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/can/io/logger.py b/can/io/logger.py index b73738827..ff689bc4e 100644 --- a/can/io/logger.py +++ b/can/io/logger.py @@ -288,8 +288,10 @@ class RotatingLogger(BaseRotatingLogger): by adding a timestamp and the rollover count. A new log file is then created and written to. - This behavior can be customized by setting the :attr:`namer` and - :attr:`rotator` attribute. + This behavior can be customized by setting the + :attr:`~can.io.BaseRotatingLogger.namer` and + :attr:`~can.io.BaseRotatingLogger.rotator` + attribute. Example:: From b8a7ec52e33962227b619e6eb3f345bc3a80f9d4 Mon Sep 17 00:00:00 2001 From: j-c-cook Date: Wed, 28 Dec 2022 11:49:22 -0600 Subject: [PATCH 14/22] Update API order to be like develop and add formats set --- can/io/logger.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/can/io/logger.py b/can/io/logger.py index ff689bc4e..868004100 100644 --- a/can/io/logger.py +++ b/can/io/logger.py @@ -340,14 +340,15 @@ class RotatingLogger(BaseRotatingLogger): :meth:`~can.Listener.stop` is called. """ + _supported_formats = {".asc", ".blf", ".csv", ".log", ".txt"} last_rollover_time = time.time() def __init__( self, base_filename: StringPathLike, - *args: Any, max_bytes: int = 0, max_seconds: int = 0, + *args: Any, **kwargs: Any, ) -> None: """ @@ -483,4 +484,4 @@ def __init__( DeprecationWarning, ) # Initialize self as a RotatingLogger - super().__init__(base_filename, *args, max_bytes=max_bytes, **kwargs) + super().__init__(base_filename, max_bytes=max_bytes, *args, **kwargs) From ce815d611d05dfe19bff722da3ea2012586207b4 Mon Sep 17 00:00:00 2001 From: j-c-cook Date: Wed, 28 Dec 2022 12:03:18 -0600 Subject: [PATCH 15/22] Fix mypy workflow build error check 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. --- can/io/logger.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/can/io/logger.py b/can/io/logger.py index 868004100..823ddc2ae 100644 --- a/can/io/logger.py +++ b/can/io/logger.py @@ -484,4 +484,4 @@ def __init__( DeprecationWarning, ) # Initialize self as a RotatingLogger - super().__init__(base_filename, max_bytes=max_bytes, *args, **kwargs) + super().__init__(base_filename, max_bytes, *args, **kwargs) From 842dcbbf9ce7d967ed074a5d8350c044659bde2c Mon Sep 17 00:00:00 2001 From: j-c-cook Date: Wed, 28 Dec 2022 12:09:56 -0600 Subject: [PATCH 16/22] Include the previous changes made to _default_name See: https://github.com/hardbyte/python-can/pull/1383 --- can/io/logger.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/can/io/logger.py b/can/io/logger.py index 823ddc2ae..776425c5f 100644 --- a/can/io/logger.py +++ b/can/io/logger.py @@ -406,12 +406,12 @@ def _default_name(self) -> StringPathLike: """Generate the default rotation filename.""" path = pathlib.Path(self.base_filename) new_name = ( - path.stem - + "_" - + datetime.now().strftime("%Y-%m-%dT%H%M%S") - + "_" - + f"#{self.rollover_count:03}" - + path.suffix + path.stem.split(".")[0] + + "_" + + datetime.now().strftime("%Y-%m-%dT%H%M%S") + + "_" + + f"#{self.rollover_count:03}" + + "".join(path.suffixes[-2:]) ) return str(path.parent / new_name) From 2a466b9e07e3e6cf26c2ffd9cfbb327f414e847b Mon Sep 17 00:00:00 2001 From: j-c-cook Date: Wed, 28 Dec 2022 12:11:39 -0600 Subject: [PATCH 17/22] Fix spacing in _default_name --- can/io/logger.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/can/io/logger.py b/can/io/logger.py index 776425c5f..a93751073 100644 --- a/can/io/logger.py +++ b/can/io/logger.py @@ -406,12 +406,12 @@ def _default_name(self) -> StringPathLike: """Generate the default rotation filename.""" path = pathlib.Path(self.base_filename) new_name = ( - path.stem.split(".")[0] - + "_" - + datetime.now().strftime("%Y-%m-%dT%H%M%S") - + "_" - + f"#{self.rollover_count:03}" - + "".join(path.suffixes[-2:]) + path.stem.split(".")[0] + + "_" + + datetime.now().strftime("%Y-%m-%dT%H%M%S") + + "_" + + f"#{self.rollover_count:03}" + + "".join(path.suffixes[-2:]) ) return str(path.parent / new_name) From 49649e7e956301c8f1977b9002b89a20f022c7a4 Mon Sep 17 00:00:00 2001 From: j-c-cook Date: Wed, 28 Dec 2022 12:15:50 -0600 Subject: [PATCH 18/22] Update logconvert to use the generalized RotatingLogger --- can/logconvert.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/can/logconvert.py b/can/logconvert.py index 730e82304..9fff873e9 100644 --- a/can/logconvert.py +++ b/can/logconvert.py @@ -6,7 +6,7 @@ import argparse import errno -from can import LogReader, Logger, SizedRotatingLogger +from can import LogReader, Logger, RotatingLogger class ArgumentParser(argparse.ArgumentParser): @@ -48,7 +48,7 @@ def main(): with LogReader(args.input) as reader: if args.file_size: - logger = SizedRotatingLogger( + logger = RotatingLogger( base_filename=args.output, max_bytes=args.file_size ) else: From 35d25c7b686dba5f650c3889cd71a7749cf6f4f1 Mon Sep 17 00:00:00 2001 From: j-c-cook Date: Wed, 28 Dec 2022 12:16:08 -0600 Subject: [PATCH 19/22] Fix comment to reference RotatingLogger --- test/test_logger.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/test_logger.py b/test/test_logger.py index 50c985fde..d4b2a6cf5 100644 --- a/test/test_logger.py +++ b/test/test_logger.py @@ -38,7 +38,7 @@ def setUp(self) -> None: self.MockLoggerUse = self.MockLogger self.loggerToUse = self.mock_logger - # Patch SizedRotatingLogger object + # Patch RotatingLogger object patcher_logger_sized = mock.patch("can.logger.RotatingLogger", spec=True) self.MockLoggerSized = patcher_logger_sized.start() self.addCleanup(patcher_logger_sized.stop) From b763f4a9f177dba89d6a2f70b18172e2000e64a1 Mon Sep 17 00:00:00 2001 From: j-c-cook Date: Wed, 28 Dec 2022 18:31:50 +0000 Subject: [PATCH 20/22] Format code with black --- can/logconvert.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/can/logconvert.py b/can/logconvert.py index 9fff873e9..209587a0d 100644 --- a/can/logconvert.py +++ b/can/logconvert.py @@ -48,9 +48,7 @@ def main(): with LogReader(args.input) as reader: if args.file_size: - logger = RotatingLogger( - base_filename=args.output, max_bytes=args.file_size - ) + logger = RotatingLogger(base_filename=args.output, max_bytes=args.file_size) else: logger = Logger(filename=args.output) From 29717f4fa8c89727e480894833645c7338ac7d6c Mon Sep 17 00:00:00 2001 From: j-c-cook Date: Wed, 28 Dec 2022 12:53:35 -0600 Subject: [PATCH 21/22] Add test for time rollover condition --- test/test_rotating_loggers.py | 50 +++++++++++++++++++++++++++++++++++ 1 file changed, 50 insertions(+) diff --git a/test/test_rotating_loggers.py b/test/test_rotating_loggers.py index ad4388bf7..3bfa6cbb2 100644 --- a/test/test_rotating_loggers.py +++ b/test/test_rotating_loggers.py @@ -263,3 +263,53 @@ def test_logfile_size_context_manager(self, tmp_path): for file_path in os.listdir(tmp_path): assert os.path.getsize(os.path.join(tmp_path, file_path)) <= 1100 + + +class TestRotatingLogger: + def test_import(self): + assert hasattr(can.io, "RotatingLogger") + assert hasattr(can, "RotatingLogger") + + def test_attributes(self): + assert issubclass(can.RotatingLogger, can.io.BaseRotatingLogger) + assert hasattr(can.RotatingLogger, "namer") + assert hasattr(can.RotatingLogger, "rotator") + assert hasattr(can.RotatingLogger, "should_rollover") + assert hasattr(can.RotatingLogger, "do_rollover") + + def test_create_instance(self, tmp_path): + base_filename = "mylogfile.ASC" + max_bytes = 512 + + logger_instance = can.SizedRotatingLogger( + base_filename=tmp_path / base_filename, max_bytes=max_bytes + ) + assert Path(logger_instance.base_filename).name == base_filename + assert logger_instance.max_bytes == max_bytes + assert logger_instance.rollover_count == 0 + assert isinstance(logger_instance.writer, can.ASCWriter) + + logger_instance.stop() + + def test_should_rollover(self, tmp_path): + base_filename = "mylogfile.ASC" + max_seconds = 300 + + logger_instance = can.RotatingLogger( + base_filename=tmp_path / base_filename, max_seconds=max_seconds + ) + msg = generate_message(0x123) + do_rollover = Mock() + logger_instance.do_rollover = do_rollover + + assert logger_instance.should_rollover(msg) is False + logger_instance.on_message_received(msg) + do_rollover.assert_not_called() + + logger_instance.last_rollover_time = \ + logger_instance.last_rollover_time - max_seconds + assert logger_instance.should_rollover(msg) is True + logger_instance.on_message_received(msg) + do_rollover.assert_called() + + logger_instance.stop() From 8aff5748644edb890c14496772c42b93af00ced7 Mon Sep 17 00:00:00 2001 From: j-c-cook Date: Wed, 28 Dec 2022 13:38:44 -0600 Subject: [PATCH 22/22] Add note to preserve size rotating test functions at v5.0 --- test/test_rotating_loggers.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/test/test_rotating_loggers.py b/test/test_rotating_loggers.py index 26b20e301..cab9161f0 100644 --- a/test/test_rotating_loggers.py +++ b/test/test_rotating_loggers.py @@ -212,6 +212,8 @@ def test_create_instance(self, tmp_path): logger_instance.stop() def test_should_rollover(self, tmp_path): + # NOTE: Move to TestRotatingLogger at v5.0 release when + # SizedRotatingLogger is removed. base_filename = "mylogfile.ASC" max_bytes = 512 @@ -235,6 +237,8 @@ def test_should_rollover(self, tmp_path): logger_instance.stop() def test_logfile_size(self, tmp_path): + # NOTE: Move to TestRotatingLogger at v5.0 release when + # SizedRotatingLogger is removed. base_filename = "mylogfile.ASC" max_bytes = 1024 msg = generate_message(0x123) @@ -251,6 +255,8 @@ def test_logfile_size(self, tmp_path): logger_instance.stop() def test_logfile_size_context_manager(self, tmp_path): + # NOTE: Move to TestRotatingLogger at v5.0 release when + # SizedRotatingLogger is removed. base_filename = "mylogfile.ASC" max_bytes = 1024 msg = generate_message(0x123) @@ -291,7 +297,7 @@ def test_create_instance(self, tmp_path): logger_instance.stop() - def test_should_rollover(self, tmp_path): + def test_should_rollover_time(self, tmp_path): base_filename = "mylogfile.ASC" max_seconds = 300