[plaso] Added CompressedFileHandler for logging #1549

[plaso] Added CompressedFileHandler for logging #1549

[Joachim Metz, 2017-12-06 06:50:51]

For reviewer this CL needs some unit testing, but putting it out there for
discussion and for testing.

[Joachim Metz, 2017-12-09 10:43:05]

Code updated.

[Joachim Metz, 2017-12-09 12:02:55]

For reviewer solved the "log garbling" issue, but I still need to write a unit
test for the compressed log file handle.

Feel free to do an initial pass

[Joachim Metz, 2017-12-10 08:26:52]

Code updated.

[Joachim Metz, 2017-12-10 08:34:08]

PTAL

[onager, 2017-12-27 20:24:58]

https://codereview.appspot.com/331400043/diff/40001/plaso/lib/loggers.py
File plaso/lib/loggers.py (right):

https://codereview.appspot.com/331400043/diff/40001/plaso/lib/loggers.py#newc...
plaso/lib/loggers.py:25: """Opens the current base file with the (original) mode
and encoding.
Is this docstring still accurate? I don't see how the base file can be
"non-current", or where the mode can change from the "original".

Please clarify/clean up this docstring.

https://codereview.appspot.com/331400043/diff/40001/plaso/lib/loggers.py#newc...
plaso/lib/loggers.py:36: If the stream was not opened because 'delay' was
specified in the
Remove this comment, there's no reopening of the stream here, and delay is
always true in the constructor.

https://codereview.appspot.com/331400043/diff/40001/plaso/lib/loggers.py#newc...
plaso/lib/loggers.py:61: # Remove all possible log handlers.
Please make this comment about why this is necessary - at the moment, this just
describes what it's doing.

https://codereview.appspot.com/331400043/diff/40001/plaso/multi_processing/ta...
File plaso/multi_processing/task_engine.py (right):

https://codereview.appspot.com/331400043/diff/40001/plaso/multi_processing/ta...
plaso/multi_processing/task_engine.py:484: # logging to the main process log
file and garbling the log.
Please add a comment to mention that these handlers are then reopened after the
child process has started.

[Joachim Metz, 2017-12-28 05:39:59]

Code updated.

[Joachim Metz, 2017-12-28 05:41:17]

https://codereview.appspot.com/331400043/diff/40001/plaso/lib/loggers.py
File plaso/lib/loggers.py (right):

https://codereview.appspot.com/331400043/diff/40001/plaso/lib/loggers.py#newc...
plaso/lib/loggers.py:25: """Opens the current base file with the (original) mode
and encoding.
this was based on the file handler, docstring changed.

https://codereview.appspot.com/331400043/diff/40001/plaso/lib/loggers.py#newc...
plaso/lib/loggers.py:36: If the stream was not opened because 'delay' was
specified in the
this was based on the file handler, docstring changed.

https://codereview.appspot.com/331400043/diff/40001/plaso/lib/loggers.py#newc...
plaso/lib/loggers.py:61: # Remove all possible log handlers.
On 2017/12/27 20:24:57, onager wrote:
> Please make this comment about why this is necessary - at the moment, this
just
> describes what it's doing.

Done.

https://codereview.appspot.com/331400043/diff/40001/plaso/multi_processing/ta...
File plaso/multi_processing/task_engine.py (right):

https://codereview.appspot.com/331400043/diff/40001/plaso/multi_processing/ta...
plaso/multi_processing/task_engine.py:484: # logging to the main process log
file and garbling the log.
On 2017/12/27 20:24:58, onager wrote:
> Please add a comment to mention that these handlers are then reopened after
the
> child process has started.

Done.

[onager, 2017-12-28 09:03:19]

https://codereview.appspot.com/331400043/diff/60001/plaso/lib/loggers.py
File plaso/lib/loggers.py (right):

https://codereview.appspot.com/331400043/diff/60001/plaso/lib/loggers.py#newc...
plaso/lib/loggers.py:25: """Opens the current compressed log file.
Is "current" still relevant? Please remove if not required.

[Joachim Metz, 2017-12-28 09:13:09]

https://codereview.appspot.com/331400043/diff/60001/plaso/lib/loggers.py
File plaso/lib/loggers.py (right):

https://codereview.appspot.com/331400043/diff/60001/plaso/lib/loggers.py#newc...
plaso/lib/loggers.py:25: """Opens the current compressed log file.
On 2017/12/28 09:03:19, onager wrote:
> Is "current" still relevant? Please remove if not required.

Done.

[Joachim Metz, 2017-12-28 09:17:56]

Code updated.

[onager, 2017-12-28 10:28:35]

LGTM

[Joachim Metz, 2017-12-28 11:07:07]

Changes have been merged with master branch. To close the review and clean up
the feature branch you can run: review.py close loggers