#490: In CommandLogHandler suffixed newline, flushed logfile and added timestamp #499
Conversation
sgn4sangar
requested a deployment
to
manual-approval
GitHub Actions
Waiting
sgn4sangar
requested a deployment
to
manual-approval
GitHub Actions
Waiting
This reverts commit 053d00f.
sgn4sangar
requested a deployment
to
manual-approval
GitHub Actions
Waiting
sgn4sangar
requested a deployment
to
manual-approval
GitHub Actions
Waiting
| ) | ||
|
|
||
|
|
||
| def test_new_line(tmp_path): |
There was a problem hiding this comment.
we probably need another test, that checks if the timestamp gets prepended
| log_handler.handle_log_line("log line 1") | ||
| log_handler.handle_log_line("log line 2") | ||
| with open(log_file_path) as log_file: | ||
| line_count = sum(1 for line in log_file) |
There was a problem hiding this comment.
Interesting construction, I like it; this actually would stream the file , which is good for large files. Maybe a bit overkill, but perfectly ok.
| timestamp = cur_time.strftime("%H.%M.%S") | ||
| msecs = str(int(cur_time.microsecond / 1000)) | ||
| usecs = str(int(cur_time.microsecond % 1000)) | ||
| return timestamp + "." + msecs + "." + usecs |
There was a problem hiding this comment.
| timestamp = cur_time.strftime("%H.%M.%S") | |
| msecs = str(int(cur_time.microsecond / 1000)) | |
| usecs = str(int(cur_time.microsecond % 1000)) | |
| return timestamp + "." + msecs + "." + usecs | |
| timestamp = cur_time.strftime("%H.%M.%S.%f") | |
| return timestamp |
I don't think we need to split the microseconds more. https://docs.python.org/3.10/library/datetime.html#strftime-strptime-behavior
There was a problem hiding this comment.
removed the static method timestamp_for_logging() and used the default 6 digits microseconds
… timestamp is prepended
sgn4sangar
requested a deployment
to
manual-approval
GitHub Actions
Waiting
sgn4sangar
changed the title
test/unit/test_log_newline.py
Outdated
| log_handler.handle_log_line("log line 1") | ||
| with open(log_file_path) as log_file: | ||
| first_line = log_file.readline() | ||
| assert re.match(pattern, first_line) |
There was a problem hiding this comment.
| assert re.match(pattern, first_line) | |
| assert re.match(pattern, first_line), f"{first_line} does not match {pattern}" |
There was a problem hiding this comment.
do we have the regex_matcher already in this project?
There was a problem hiding this comment.
Indeed, we have,
, pls use that one then
|
Fixes #490