Moved worker status update out of engine for multi processing #120 #169

These files are missing unit tests:
 + plaso/engine/processing_status.py + tools/log2timeline.py

[Joachim Metz, 2015-04-26 11:11:11]

Code updated.

[Joachim Metz, 2015-04-26 17:54:32]

Code updated.

[Joachim Metz, 2015-04-26 18:24:01]

Code updated.

[kiddi, 2015-04-27 23:04:13]

Basically only one comment there

https://codereview.appspot.com/229410043/diff/60001/plaso/multi_processing/fo...
File plaso/multi_processing/foreman.py (right):

https://codereview.appspot.com/229410043/diff/60001/plaso/multi_processing/fo...
plaso/multi_processing/foreman.py:46: self._monitored_process_labels = {}
so I see you remove last_status_dict.... I think we should actually keep this
and have last status displayed too (I'm dropping
https://codereview.appspot.com/224610044/ for now) but take a look at the "diff
from last run" aspect of the foreman changes there.

When you see X events extracted it doesn't really tell you a lot, but if you see
X events extracted, which is Y different from last run, you can tell pretty
quickly if any given worker is still really working and producing events, so I
think that is an important piece of information to retain, whether that is by
keeping the entire status dict, or just the last count for each parser and pass
that to the status message.

[Joachim Metz, 2015-04-28 06:00:51]

Ack I'll see if I can add this to the processing status. I also need to fix the
end of input assumptions errors.

[Joachim Metz, 2015-04-28 06:01:01]

https://codereview.appspot.com/229410043/diff/60001/plaso/multi_processing/fo...
File plaso/multi_processing/foreman.py (right):

https://codereview.appspot.com/229410043/diff/60001/plaso/multi_processing/fo...
plaso/multi_processing/foreman.py:46: self._monitored_process_labels = {}
Having this functionality SGTM but IMO we should move that to the processing
status.

[Joachim Metz, 2015-04-28 16:53:58]

Code updated.

[Joachim Metz, 2015-04-28 17:05:42]

Code updated.

[Joachim Metz, 2015-04-30 08:07:54]

https://codereview.appspot.com/229410043/diff/100001/plaso/multi_processing/f...
File plaso/multi_processing/foreman.py (right):

https://codereview.appspot.com/229410043/diff/100001/plaso/multi_processing/f...
plaso/multi_processing/foreman.py:226:
self.processing_status.number_of_path_specs == number_of_path_specs):
make sure to take path specs generated in the workers into account.

[onager, 2015-05-03 10:56:59]

https://codereview.appspot.com/229410043/diff/100001/plaso/multi_processing/f...
File plaso/multi_processing/foreman.py (right):

https://codereview.appspot.com/229410043/diff/100001/plaso/multi_processing/f...
plaso/multi_processing/foreman.py:17: The foreman is responsible for monitoring
processes and give back status
not give, giving or reporting

[kiddi, 2015-05-04 16:27:07]

Few comments

https://codereview.appspot.com/229410043/diff/100001/plaso/analysis/interface.py
File plaso/analysis/interface.py (right):

https://codereview.appspot.com/229410043/diff/100001/plaso/analysis/interface...
plaso/analysis/interface.py:72: self.ExamineEvent(analysis_mediator, item,
**kwargs)
if you are changing this to ConsumeItem, shouldn't this then be changes as well
to ExamineItem instead of ExamineEvent?

https://codereview.appspot.com/229410043/diff/100001/plaso/engine/processing_...
File plaso/engine/processing_status.py (right):

https://codereview.appspot.com/229410043/diff/100001/plaso/engine/processing_...
plaso/engine/processing_status.py:44: for extraction_worker_status in
self._extraction_workers.values():
.itervalues() here

https://codereview.appspot.com/229410043/diff/100001/plaso/engine/processing_...
plaso/engine/processing_status.py:51: for extraction_worker_status in
self._extraction_workers.values():
itervalues()

https://codereview.appspot.com/229410043/diff/100001/plaso/frontend/plasm_tes...
File plaso/frontend/plasm_test.py (right):

https://codereview.appspot.com/229410043/diff/100001/plaso/frontend/plasm_tes...
plaso/frontend/plasm_test.py:50: test_queue =
multi_process.MultiProcessingQueue(timeout=0.1)
any particular reason you are setting a timeout value here? And why this
particular value?

https://codereview.appspot.com/229410043/diff/100001/plaso/multi_processing/f...
File plaso/multi_processing/foreman.py (right):

https://codereview.appspot.com/229410043/diff/100001/plaso/multi_processing/f...
plaso/multi_processing/foreman.py:60: This function takes a process label and
checks if the corresponting
corresponding

[Joachim Metz, 2015-05-05 14:27:12]

https://codereview.appspot.com/229410043/diff/100001/plaso/analysis/interface.py
File plaso/analysis/interface.py (right):

https://codereview.appspot.com/229410043/diff/100001/plaso/analysis/interface...
plaso/analysis/interface.py:72: self.ExamineEvent(analysis_mediator, item,
**kwargs)
On 2015/05/04 16:27:07, kiddi wrote:
> if you are changing this to ConsumeItem, shouldn't this then be changes as
well
> to ExamineItem instead of ExamineEvent?

We could do that though I largely only wanted merge the different consumer
implementations for now. I can add a todo.

https://codereview.appspot.com/229410043/diff/100001/plaso/engine/processing_...
File plaso/engine/processing_status.py (right):

https://codereview.appspot.com/229410043/diff/100001/plaso/engine/processing_...
plaso/engine/processing_status.py:44: for extraction_worker_status in
self._extraction_workers.values():
On 2015/05/04 16:27:07, kiddi wrote:
> .itervalues() here

Done.

https://codereview.appspot.com/229410043/diff/100001/plaso/engine/processing_...
plaso/engine/processing_status.py:51: for extraction_worker_status in
self._extraction_workers.values():
On 2015/05/04 16:27:07, kiddi wrote:
> itervalues()

Done.

https://codereview.appspot.com/229410043/diff/100001/plaso/frontend/plasm_tes...
File plaso/frontend/plasm_test.py (right):

https://codereview.appspot.com/229410043/diff/100001/plaso/frontend/plasm_tes...
plaso/frontend/plasm_test.py:50: test_queue =
multi_process.MultiProcessingQueue(timeout=0.1)
On 2015/05/04 16:27:07, kiddi wrote:
> any particular reason you are setting a timeout value here? And why this
> particular value?

Done.

https://codereview.appspot.com/229410043/diff/100001/plaso/multi_processing/f...
File plaso/multi_processing/foreman.py (right):

https://codereview.appspot.com/229410043/diff/100001/plaso/multi_processing/f...
plaso/multi_processing/foreman.py:17: The foreman is responsible for monitoring
processes and give back status
On 2015/05/03 10:56:59, onager wrote:
> not give, giving or reporting

Done.

https://codereview.appspot.com/229410043/diff/100001/plaso/multi_processing/f...
plaso/multi_processing/foreman.py:60: This function takes a process label and
checks if the corresponting
On 2015/05/04 16:27:07, kiddi wrote:
> corresponding 

Done.

https://codereview.appspot.com/229410043/diff/100001/plaso/multi_processing/f...
plaso/multi_processing/foreman.py:226:
self.processing_status.number_of_path_specs == number_of_path_specs):
On 2015/04/30 08:07:53, Joachim Metz wrote:
> make sure to take path specs generated in the workers into account.

Done.

[Joachim Metz, 2015-05-05 14:31:52]

Code updated.

[kiddi, 2015-05-06 09:28:58]

LGTM

https://codereview.appspot.com/229410043/diff/100001/plaso/analysis/interface.py
File plaso/analysis/interface.py (right):

https://codereview.appspot.com/229410043/diff/100001/plaso/analysis/interface...
plaso/analysis/interface.py:72: self.ExamineEvent(analysis_mediator, item,
**kwargs)
On 2015/05/05 14:27:11, Joachim Metz wrote:
> On 2015/05/04 16:27:07, kiddi wrote:
> > if you are changing this to ConsumeItem, shouldn't this then be changes as
> well
> > to ExamineItem instead of ExamineEvent?
> 
> We could do that though I largely only wanted merge the different consumer
> implementations for now. I can add a todo.

yes, let's add a TODO at leat

[Joachim Metz, 2015-05-06 10:59:27]

Code Submitted.