Skip to content

Modify file_size help doc string #1401

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.

Sign up for GitHub

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

Merged
merged 12 commits into from
Oct 7, 2022
Merged

Modify file_size help doc string #1401

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
6f6c325
Modify `file_size` help doc string
Sep 29, 2022
18241ac
Add note to BLFWriter
Sep 29, 2022
b26fd57
Revert "Add note to BLFWriter"
Sep 29, 2022
a4d04dc
Add in a note about the nuanced file size in help statement
Sep 29, 2022
cced4ad
Modify help statement to clarify consistency
Sep 29, 2022
87ecc64
Minor tweak
Sep 29, 2022
c39719c
Control BLFWriter max container size
Sep 29, 2022
3afb140
Fix formatting
Sep 29, 2022
42c06be
Update can/logger.py
j-c-cook Oct 3, 2022
6fa459d
Revert "Fix formatting"
Oct 3, 2022
053ead4
Revert "Control BLFWriter max container size"
Oct 3, 2022
442d484
Merge branch 'LoggerHelpStatement' of https://github.com/j-c-cook/pyt…
Oct 3, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions can/io/logger.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -325,6 +325,12 @@ def __init__(
self.base_filename = os.path.abspath(base_filename)
self.max_bytes = max_bytes

# BLFWriter specific
_128_kb = 128 * 1024
if kwargs.get('max_container_size', 1) and max_bytes > 0:
kwargs['max_container_size'] = \
_128_kb if max_bytes > _128_kb else max_bytes * 0.1

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@zariiii9003 This would get rid of the nuance. I'm anticipating that you are not going to like the BLFWriter specific code inside of the SizedRotatingLogger constructor, but let me know.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, i don't like this here 😄
Also i do not understand what this does tbh, what's the purpose of this? And why is the default value of kwargs.get('max_container_size', 1) =1?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If max_container_size is not defined then set the max_container_size. If max_container_size is not defined then set the value to be 128 kb unless the file_size is less than 128 kb. In that case set the max_container_size to 10% of the file_size.

I'm reverting the commits because I agree that its not a good location.

self._writer = self._get_new_writer(self.base_filename)

def should_rollover(self, msg: Message) -> bool:
Expand Down
7 changes: 4 additions & 3 deletions can/logger.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -193,9 +193,10 @@ def main() -> None:
"--file_size",
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.",
help="Maximum file size in bytes. Rotate log file when size threshold "
"is reached. (The resulting file sizes will be consistent, but is not "
"guaranteed to be exactly what is specified here due to the rollover "
"conditions being logger implementation specific.)",
default=None,
)

Expand Down